You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2020/04/16 14:50:48 UTC

[GitHub] [httpcomponents-client] heejeongkim opened a new pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge

heejeongkim opened a new pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge
URL: https://github.com/apache/httpcomponents-client/pull/222
 
 
   https://issues.apache.org/jira/browse/HTTPCLIENT-2076

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-client] ok2c commented on a change in pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge
URL: https://github.com/apache/httpcomponents-client/pull/222#discussion_r409809666
 
 

 ##########
 File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/BasicMaxAgeHandler.java
 ##########
 @@ -53,18 +53,16 @@ public void parse(final SetCookie cookie, final String value)
             throws MalformedCookieException {
         Args.notNull(cookie, "Cookie");
         if (value == null) {
-            throw new MalformedCookieException("Missing value for 'max-age' attribute");
+            return;
         }
         final int age;
         try {
             age = Integer.parseInt(value);
         } catch (final NumberFormatException e) {
-            throw new MalformedCookieException ("Invalid 'max-age' attribute: "
 
 Review comment:
   @heejeongkim I am quite sure the statement in the section 4.1.2 refers to custom attributes, say `custom-stuff`, not malformed standard ones. I think it would be wrong to silently ignore standard attribute values that are obviously malformed.
   
   Let's fix the NPE in this PR and decide what should be done about `LaxExpiresHandler` in a new one. I prefer to not change the behavior of `BasicExpiresHandler` and `BasicMaxAgeHandler` at all.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-client] heejeongkim commented on a change in pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge

Posted by GitBox <gi...@apache.org>.
heejeongkim commented on a change in pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge
URL: https://github.com/apache/httpcomponents-client/pull/222#discussion_r409788229
 
 

 ##########
 File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/BasicMaxAgeHandler.java
 ##########
 @@ -53,18 +53,16 @@ public void parse(final SetCookie cookie, final String value)
             throws MalformedCookieException {
         Args.notNull(cookie, "Cookie");
         if (value == null) {
-            throw new MalformedCookieException("Missing value for 'max-age' attribute");
+            return;
         }
         final int age;
         try {
             age = Integer.parseInt(value);
         } catch (final NumberFormatException e) {
-            throw new MalformedCookieException ("Invalid 'max-age' attribute: "
 
 Review comment:
   @ok2c [rfc6265#section-4.1.2](https://tools.ietf.org/html/rfc6265#section-4.1.2) states that '_User agents ignore unrecognized cookie attributes (but not the entire cookie)._' but If we throw MalformedCookieException while parsing invalid ```max-age``` / ```expires``` attribute, the entire set-cookie-string containing invalid attribute is failed to parse.
   
   ```java
   URL url = new URL("https://www.webmd.com/heartburn-gerd/news/20131210/acid-reflux-drugs-tied-to-lower-levels-of-vitamin-b-12"); 
   CookieOrigin origin = new CookieOrigin(url.getHost(),
               url.getPort() > 0 ? url.getPort() : url.getDefaultPort(),
               url.getPath(),
               url.getProtocol().equalsIgnoreCase("https"));
   Header header = new BasicHeader("Set-Cookie", "gtinfo=info; Expirest=garbage; domain=.webmd.com; path=/");
   CookieSpec cookieSpec = new RFC6265StrictSpec();
   List<Cookie> cookies = cookieSpec.parse(header, origin); // MalformedCookieException is thrown in this line. So user cannot retrieve Cookie from the set-cookie string
   ```
   
   Do you think I need to open a separate issue for this like Michael Osipov suggested? ([link](https://issues.apache.org/jira/browse/HTTPCLIENT-2076?focusedCommentId=17085053&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17085053))

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-client] ok2c merged pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge

Posted by GitBox <gi...@apache.org>.
ok2c merged pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge
URL: https://github.com/apache/httpcomponents-client/pull/222
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-client] ok2c commented on a change in pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #222: HTTPCLIENT-2076: fix NPE in LaxExpiresHandler and ignore invalid Expires / MaxAge
URL: https://github.com/apache/httpcomponents-client/pull/222#discussion_r409742770
 
 

 ##########
 File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/cookie/BasicMaxAgeHandler.java
 ##########
 @@ -53,18 +53,16 @@ public void parse(final SetCookie cookie, final String value)
             throws MalformedCookieException {
         Args.notNull(cookie, "Cookie");
         if (value == null) {
-            throw new MalformedCookieException("Missing value for 'max-age' attribute");
+            return;
         }
         final int age;
         try {
             age = Integer.parseInt(value);
         } catch (final NumberFormatException e) {
-            throw new MalformedCookieException ("Invalid 'max-age' attribute: "
 
 Review comment:
   @heejeongkim Why do you want to quietly ignore malformed `max-age` and `expires` attributes with strict RFC 6265 policy? That does not look right to me.   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org