rzo1 commented on code in PR #2075:
URL: https://github.com/apache/stormcrawler/pull/2075#discussion_r3879008749
##########
core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:
##########
@@ -286,8 +286,10 @@ private void addCookiesToRequest(Builder rb, String url,
Metadata md) {
return;
}
try {
+ // the host whose response set the cookies is not kept in the
metadata,
+ // hence the null origin: cookies without a domain attribute are
dropped
final List<Cookie> cookies =
- CookieConverter.getCookies(cookieStrings,
URLUtil.toURL(url));
+ CookieConverter.getCookies(cookieStrings, null,
URLUtil.toURL(url));
Review Comment:
Recorded rather than deferred. When a response carries `Set-Cookie`, okhttp
and playwright write its URL alongside as `set-cookie-origin`, dropping any
inbound key of that name first so a response cannot pick where its cookies go.
`addCookiesToRequest` reads it back with the protocol prefix. It has to travel
with `protocol.set-cookie` in `metadata.transfer`, so it is in the examples and
a warning naming both keys is logged once when cookies arrive without it; the
cookies paragraph in `internals.adoc` is fixed too. Host-only cookies
round-trip in the new `HttpProtocolCookieTest`, including via a 302.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
