Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the inconsistency in overwriting the identical cookie #1907

Merged
merged 12 commits into from
Mar 20, 2024

Conversation

TharmiganK
Copy link
Contributor

@TharmiganK TharmiganK commented Mar 20, 2024

Purpose

$Subject

Fixes: ballerina-platform/ballerina-library#6194

Examples

  • Service code
service /api on new http:Listener(9090) {

    resource function get foo(string value = "foo") returns http:Response {
        http:Response res = new;
        http:Cookie resCookie = new ("cookie", value, path = "/api/foo");
        res.addCookie(resCookie);
        res.setTextPayload("Hello, World!");
        return res;
    }
}
  • Client code
import ballerina/http;

public function main() returns error? {
    final http:Client cookieClient = check new ("localhost:9090",
        cookieConfig = {
            enabled: true
        },
        httpVersion = http:HTTP_1_1
    );

    // First request
    http:Response _ = check cookieClient->/api/foo;

    // Second request
    http:Response _ = check cookieClient->/api/foo(value = "bar");

    // Third request
    http:Response _ = check cookieClient->/api/foo;
}

Behaviour can be analysed with trace logs:

Client's first request - without any cookies & response

GET /api/foo HTTP/1.1
host: localhost:9090
user-agent: ballerina
connection: keep-alive 

HTTP/1.1 200 OK
Set-Cookie: cookie=foo
content-length: 0
server: ballerina

Client's second request - with cookie & response

GET /api/foo?value=bar HTTP/1.1
Cookie: cookie=foo
host: localhost:9090
user-agent: ballerina
connection: keep-alive 

HTTP/1.1 200 OK
Set-Cookie: cookie=bar
content-length: 0
server: ballerina

Client's third request - with overwritten cookie & response

GET /api/foo HTTP/1.1
Cookie: cookie=bar
host: localhost:9090
user-agent: ballerina
connection: keep-alive 

HTTP/1.1 200 OK
Set-Cookie: cookie=foo
content-length: 0
server: ballerina

Checklist

  • Linked to an issue
  • Updated the changelog
  • Added tests
  • Updated the spec
  • Checked native-image compatibility
  • Checked the impact on OpenAPI generation

Co-authored-by: Dilan Sachintha Nayanajith <[email protected]>
Copy link

sonarcloud bot commented Mar 20, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 96.29630% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 81.67%. Comparing base (31bc185) to head (ff0155d).
Report is 1 commits behind head on master.

Files Patch % Lines
ballerina/cookie_utils.bal 91.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1907      +/-   ##
============================================
- Coverage     81.70%   81.67%   -0.04%     
  Complexity      562      562              
============================================
  Files           393      393              
  Lines         21513    21513              
  Branches       4801     4801              
============================================
- Hits          17578    17570       -8     
- Misses         2942     2945       +3     
- Partials        993      998       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TharmiganK TharmiganK merged commit b91eb3a into master Mar 20, 2024
8 checks passed
@TharmiganK TharmiganK deleted the identical-cookie-fix branch March 20, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Http Client Cookies is not maintained in consistent manner
2 participants