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

MockCookie parsing fails if cookie does not include attributes [SPR-17321] #21854

Closed
spring-projects-issues opened this issue Oct 1, 2018 · 6 comments
Assignees
Labels
in: test type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Oct 1, 2018

Michael Bell opened SPR-17321 and commented

In brief, the MockServletRequest and MockServletResponse cannot handle a cookie of name=value . They require name=value; (optionalattribute). Without the semi-colon, MockCookie.parse(), introduced in Spring 5.1 RC2 throws an ArrayIndexOutOfBoundsException.

@Test(expected = ArrayIndexOutOfBoundsException.class)
public void springTestParsingBug() {
    // Demonstrates Spring can't handle this
    MockCookie.parse("a=1");
}

@Test
public void springTestParsingBugWorkaround() {
    // Demonstrates Spring test needs an attribute
    MockCookie.parse("a=1; HttpOnly");
}

@Test
public void springTestParsingBugWorkaround2() {
    // Demonstrates Spring test needs an attribute
    MockCookie.parse("a=1;");
}

Regression was introduced in the work discussed in #21647.

 


Affects: 5.1 GA

Issue Links:

  • #21647 Add support for adding cookies as headers in MockHttpServletResponse
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 1, 2018

Michael Bell commented

Relevant buggy code snippet

public static MockCookie parse(String setCookieHeader) {
   String[] cookieParts = setCookieHeader.split("\\s*=\\s*", 2);
   Assert.isTrue(cookieParts.length == 2, "Invalid Set-Cookie header value");

   String name = cookieParts[0];
   String[] valueAndDirectives = cookieParts[1].split("\\s*;\\s*", 2);
   String value = valueAndDirectives[0];
// This line will throw the error if no semicolon is present
   String[] directives = valueAndDirectives[1].split("\\s*;\\s*");

   MockCookie cookie = new MockCookie(name, value);

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 1, 2018

Michael Bell commented

Relevant part of RFC

 

set-cookie-header = "Set-Cookie:" SP set-cookie-string
 set-cookie-string = cookie-pair *( ";" SP cookie-av )
 cookie-pair       = cookie-name "=" cookie-value

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 1, 2018

Michael Bell commented

I believe the "simple fix" is

String[] directives = valueAndDirectives.length > 1 ? valueAndDirectives[1].split("\\s*;\\s*") : new String[0];

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 1, 2018

Michael Bell commented

#1974

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 2, 2018

Sam Brannen commented

Thanks for reporting the issue.

We'll look into it.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 2, 2018

Sam Brannen commented

Resolving this issue, since the PR has been merged into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test type: bug
Projects
None yet
Development

No branches or pull requests

2 participants