-
-
Notifications
You must be signed in to change notification settings - Fork 727
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
authhelper: cookie handling tweak + session bug fix #6249
Conversation
addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/ExtensionAuthhelper.java
Show resolved
Hide resolved
|
||
Map<String, String> trackedCookies = | ||
Arrays.asList(hbSession.getHttpState().getCookies()).stream() | ||
.collect(Collectors.toMap(c -> c.getName(), c -> c.getValue())); | ||
|
||
List<HttpCookie> cookies = message.getRequestHeader().getHttpCookies(); | ||
for (Pair<String, String> header : hbSession.getHeaders()) { | ||
if (HttpHeader.COOKIE.equalsIgnoreCase(header.first)) { | ||
String[] kv = header.second.split("="); | ||
if (!trackedCookies.containsKey(kv[0])) { | ||
cookies.add(new HttpCookie(kv[0], kv[1])); | ||
} else { | ||
LOGGER.debug( | ||
"processMessageToMatchSession {} ignoring tracked cookie {} ", | ||
message.getRequestHeader().getURI(), | ||
kv[0]); | ||
} | ||
continue; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this use CookieUtils or something instead of kinda re-inventing the wheel so to speak?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What part is reinventing the wheel?
Its doing some very specific things here, and I couldnt see any existing code which does the same things.
Very happy to use existing code if you can point out anything suitable 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the splitting and array handling. I agree it isn't complex code I just think we already have handling in commonlib, maybe I'm wrong and not digging enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The values returned from HttpHeaderBasedSession.getHeaders() are not strictly speaking headers, they are config values. If the first string is HttpHeader.COOKIE then the value should be key=value, not a full cookie header string.
We could use CookieUtils to parse it, but I dont think thats strictly necessary or even strictly correct.
Signed-off-by: Simon Bennetts <[email protected]>
f0bb8b3
to
8b40589
Compare
Thank you! |
Great job, no security vulnerabilities found in this Pull Request |
Overview
Checklist
./gradlew spotlessApply
for code formattingFor more details, please refer to the developer rules and guidelines.