commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jsde...@apache.org
Subject cvs commit: jakarta-commons/httpclient/src/java/org/apache/commons/httpclient Cookie.java HttpMethodBase.java
Date Mon, 02 Dec 2002 06:14:56 GMT
jsdever     2002/12/01 22:14:56

  Modified:    httpclient/src/java/org/apache/commons/httpclient
                        Cookie.java HttpMethodBase.java
  Log:
  Improve logging for Cookie class.
  
   Reduced the amount of output in the logs caused by rejection of
   an invalid cookie.
  
   Changes:
  
   1) I find a full call stack dump is a bit too much for a such a matter
   as rejected cookie. I took liberty in removing it
  
   2) Invalid cookie is reported only once in the HttpMethodBase class with
   WARN priority
  
   3) Exception messages are less wordy
  
  Contributed by: Oleg Kalnichevski
  
  Revision  Changes    Path
  1.28      +138 -147  jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/Cookie.java
  
  Index: Cookie.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/Cookie.java,v
  retrieving revision 1.27
  retrieving revision 1.28
  diff -u -r1.27 -r1.28
  --- Cookie.java	29 Nov 2002 19:03:02 -0000	1.27
  +++ Cookie.java	2 Dec 2002 06:14:56 -0000	1.28
  @@ -807,152 +807,143 @@
               defaultPath = path;
           }
           
  -        try {
  -            HeaderElement[] headerElements =
  -                HeaderElement.parse(setCookie.getValue());
  -    
  -            Cookie[] cookies = new Cookie[headerElements.length];
  -    
  -            int index = 0;
  -            for (int i = 0; i < headerElements.length; i++) {
  -    
  -                HeaderElement headerelement = headerElements[i];
  -                Cookie cookie = new Cookie(domain,
  -                                           headerelement.getName(),
  -                                           headerelement.getValue(),
  -                                           defaultPath, 
  -                                           null,
  -                                           false);
  -    
  -                // cycle through the parameters
  -                NameValuePair[] parameters = headerelement.getParameters();
  -                // could be null. In case only a header element and no parameters.
  -                if (parameters != null) {
  -    
  -                    for (int j = 0; j < parameters.length; j++) {
  -    
  -                        String param_name = parameters[j].getName().toLowerCase();
  -                        String param_value = parameters[j].getValue();
  -    
  -                        if (param_name.equals("version")) {
  -    
  -                            if (param_value == null) {
  -                                throw new HttpException("Bad cookie header: missing value
for version attribute");
  -                            }
  -                            try {
  -                               cookie.setVersion(Integer.parseInt(param_value));
  -                            } catch (NumberFormatException e) {
  -                                throw new HttpException( "Bad cookie header: invalid version
attribute: " + e.getMessage());
  -                            }
  -    
  -                        } 
  -                        else if (param_name.equals("path")) {
  -    
  -                            if (param_value == null) {
  -                                throw new HttpException("Bad cookie header: missing value
for path attribute");
  -                            }
  -                            cookie.setPath(param_value);
  -                            cookie.setPathAttributeSpecified(true);
  -    
  -                        } 
  -                        else if (param_name.equals("domain")) {
  -    
  -                            if (param_value == null) {
  -                                throw new HttpException("Bad cookie header: missing value
for domain attribute");
  -                            }
  -                            cookie.setDomain(param_value);
  -                            cookie.setDomainAttributeSpecified(true);
  -    
  -                        } 
  -                        else if (param_name.equals("max-age")) {
  -    
  -                            if (param_value == null) {
  -                                throw new HttpException("Bad cookie header: missing value
for max-age attribute");
  -                            }
  -                            int age;
  +        HeaderElement[] headerElements =
  +            HeaderElement.parse(setCookie.getValue());
  +
  +        Cookie[] cookies = new Cookie[headerElements.length];
  +
  +        int index = 0;
  +        for (int i = 0; i < headerElements.length; i++) {
  +
  +            HeaderElement headerelement = headerElements[i];
  +            Cookie cookie = new Cookie(domain,
  +                                       headerelement.getName(),
  +                                       headerelement.getValue(),
  +                                       defaultPath, 
  +                                       null,
  +                                       false);
  +
  +            // cycle through the parameters
  +            NameValuePair[] parameters = headerelement.getParameters();
  +            // could be null. In case only a header element and no parameters.
  +            if (parameters != null) {
  +
  +                for (int j = 0; j < parameters.length; j++) {
  +
  +                    String param_name = parameters[j].getName().toLowerCase();
  +                    String param_value = parameters[j].getValue();
  +
  +                    if (param_name.equals("version")) {
  +
  +                        if (param_value == null) {
  +                            throw new HttpException("Missing value for version attribute");
  +                        }
  +                        try {
  +                           cookie.setVersion(Integer.parseInt(param_value));
  +                        } catch (NumberFormatException e) {
  +                            throw new HttpException( "Invalid version attribute: " + e.getMessage());
  +                        }
  +
  +                    } 
  +                    else if (param_name.equals("path")) {
  +
  +                        if (param_value == null) {
  +                            throw new HttpException("Missing value for path attribute");
  +                        }
  +                        cookie.setPath(param_value);
  +                        cookie.setPathAttributeSpecified(true);
  +
  +                    } 
  +                    else if (param_name.equals("domain")) {
  +
  +                        if (param_value == null) {
  +                            throw new HttpException("Missing value for domain attribute");
  +                        }
  +                        cookie.setDomain(param_value);
  +                        cookie.setDomainAttributeSpecified(true);
  +
  +                    } 
  +                    else if (param_name.equals("max-age")) {
  +
  +                        if (param_value == null) {
  +                            throw new HttpException("Missing value for max-age attribute");
  +                        }
  +                        int age;
  +                        try {
  +                            age = Integer.parseInt(param_value);
  +                        } catch (NumberFormatException e) {
  +                            throw new HttpException( "Invalid max-age attribute: " + e.getMessage());
  +                        }
  +                        cookie.setExpiryDate(new Date(System.currentTimeMillis() +
  +                                age * 1000L));
  +
  +                    } 
  +                    else if (param_name.equals("secure")) {
  +
  +                        cookie.setSecure(true);
  +
  +                    } 
  +                    else if (param_name.equals("comment")) {
  +
  +                        cookie.setComment(param_value);
  +
  +                    } 
  +                    else if (param_name.equals("expires")) {
  +
  +                        if (param_value == null) {
  +                            throw new HttpException("Missing value for expires attribute");
  +                        }
  +                        boolean set = false;
  +                        // trim single quotes around expiry if present
  +                        // see http://nagoya.apache.org/bugzilla/show_bug.cgi?id=5279
  +                        if(param_value.length() > 1 &&
  +                                param_value.startsWith("'") &&
  +                                param_value.endsWith("'")) {
  +                            param_value = param_value.substring(1,param_value.length()-1);
  +                        }
  +
  +                        for(int k=0;k<expiryFormats.length;k++) {
  +
                               try {
  -                                age = Integer.parseInt(param_value);
  -                            } catch (NumberFormatException e) {
  -                                throw new HttpException( "Bad cookie header: invalid max-age
attribute: " + e.getMessage());
  -                            }
  -                            cookie.setExpiryDate(new Date(System.currentTimeMillis() +
  -                                    age * 1000L));
  -    
  -                        } 
  -                        else if (param_name.equals("secure")) {
  -    
  -                            cookie.setSecure(true);
  -    
  -                        } 
  -                        else if (param_name.equals("comment")) {
  -    
  -                            cookie.setComment(param_value);
  -    
  -                        } 
  -                        else if (param_name.equals("expires")) {
  -    
  -                            if (param_value == null) {
  -                                throw new HttpException("Bad cookie header: missing value
for expires attribute");
  -                            }
  -                            boolean set = false;
  -                            // trim single quotes around expiry if present
  -                            // see http://nagoya.apache.org/bugzilla/show_bug.cgi?id=5279
  -                            if(param_value.length() > 1 &&
  -                                    param_value.startsWith("'") &&
  -                                    param_value.endsWith("'")) {
  -                                param_value = param_value.substring(1,param_value.length()-1);
  -                            }
  -    
  -                            for(int k=0;k<expiryFormats.length;k++) {
  -    
  -                                try {
  -                                    Date date = expiryFormats[k].parse(param_value);
  -                                    cookie.setExpiryDate(date);
  -                                    set = true;
  -                                    break;
  -                                } catch (ParseException e) {
  -                                    //Ignore and move on
  -                                }
  -                            }
  -                            if(!set) {
  -                                throw new HttpException("Bad cookie header: unable to parse
expiration date parameter: " + param_value);
  +                                Date date = expiryFormats[k].parse(param_value);
  +                                cookie.setExpiryDate(date);
  +                                set = true;
  +                                break;
  +                            } catch (ParseException e) {
  +                                //Ignore and move on
                               }
                           }
  +                        if(!set) {
  +                            throw new HttpException("Unable to parse expiration date parameter:
" + param_value);
  +                        }
                       }
                   }
  -                
  -                // Check if the cookie has all required attributes. If not, apply defaults
  -                if (cookie.getDomain() == null) {
  -                    cookie.setDomain(domain);
  -                    cookie.setDomainAttributeSpecified(false);
  -                    if (log.isInfoEnabled()) {
  -                        log.info("Domain attribute not explicitly set. Default applied:
\"" + domain + "\"");
  -                    }
  -                }
  -                if (cookie.getPath() == null) {
  -                    cookie.setPath(defaultPath);
  -                    cookie.setPathAttributeSpecified(false);
  -                    if (log.isInfoEnabled()) {
  -                        log.info("Path attribute not explicitly set. Default applied: \""
+ defaultPath + "\"");
  -                    }
  +            }
  +            
  +            // Check if the cookie has all required attributes. If not, apply defaults
  +            if (cookie.getDomain() == null) {
  +                cookie.setDomain(domain);
  +                cookie.setDomainAttributeSpecified(false);
  +                if (log.isDebugEnabled()) {
  +                    log.debug("Domain attribute not explicitly set. Default applied: \""
+ domain + "\"");
                   }
  -    
  -                validate(cookie, domain, port, path, secure);
  -                
  -                if(log.isDebugEnabled()){
  -                    log.debug("Cookie accepted: \"" + cookie.toString() +"\"");
  +            }
  +            if (cookie.getPath() == null) {
  +                cookie.setPath(defaultPath);
  +                cookie.setPathAttributeSpecified(false);
  +                if (log.isDebugEnabled()) {
  +                    log.debug("Path attribute not explicitly set. Default applied: \""
+ defaultPath + "\"");
                   }
  -                cookies[index++] = cookie;
               }
  -            return cookies;
  -        }
  -        catch(HttpException e)
  -        {
  -            if (log.isInfoEnabled()) {
  -                log.info("Cookie rejected: \"" + setCookie.getValue() + "\". " + e.getMessage());
  +
  +            validate(cookie, domain, port, path, secure);
  +            
  +            if(log.isDebugEnabled()){
  +                log.debug("Cookie accepted: \"" + cookie.toString() +"\"");
               }
  -            throw e;
  +            cookies[index++] = cookie;
           }
  +        return cookies;
       }
   
   
  @@ -964,7 +955,7 @@
   
           // check version
           if (cookie.getVersion() < 0) {
  -            throw new HttpException( "Bad cookie header: illegal version number " + cookie.getValue());
  +            throw new HttpException( "Illegal version number " + cookie.getValue());
           }
   
           // security check... we musn't allow the server to give us an
  @@ -983,7 +974,7 @@
               if (!domain.endsWith(cookie.getDomain())){
            
                   throw new HttpException(
  -                    "Bad cookie header: illegal domain attribute \"" + cookie.getDomain()
+ "\". Domain of origin: \"" + domain + "\"");
  +                    "Illegal domain attribute \"" + cookie.getDomain() + "\". Domain of
origin: \"" + domain + "\"");
            
               }
   
  @@ -1004,7 +995,7 @@
           if(!path.startsWith(cookie.getPath())) {
   
                   throw new HttpException(
  -                    "Bad cookie header: illegal path attribute \"" + cookie.getPath() +
"\". Path of origin: \"" + path + "\"");
  +                    "Illegal path attribute \"" + cookie.getPath() + "\". Path of origin:
\"" + path + "\"");
           }
       }
   
  @@ -1022,14 +1013,14 @@
               
               if(domainParts < 2) {
   
  -                throw new HttpException("Bad cookie header: domain attribute \""+ cookie.getDomain()
+ "\" violates the Netscape cookie specification for special domains");
  +                throw new HttpException("Domain attribute \""+ cookie.getDomain() + "\"
violates the Netscape cookie specification for special domains");
               }
           }
           else {
   
               if(domainParts < 3) {
   
  -                throw new HttpException("Bad cookie header: domain attribute \""+ cookie.getDomain()
+ "\" violates the Netscape cookie specification");
  +                throw new HttpException("Domain attribute \""+ cookie.getDomain() + "\"
violates the Netscape cookie specification");
               }            
           }
       }
  @@ -1046,7 +1037,7 @@
           int dotIndex = cookie.getDomain().indexOf('.', 1);
           if(dotIndex < 0 || dotIndex == cookie.getDomain().length()-1){
   
  -                throw new HttpException("Bad cookie header: illegal domain attribute \""
+ cookie.getDomain() + "\"");
  +                throw new HttpException("Illegal domain attribute \"" + cookie.getDomain()
+ "\"");
   
           }
           // host minus domain may not contain any dots
  @@ -1054,7 +1045,7 @@
                   domain.length() -
                   cookie.getDomain().length()).indexOf('.') != -1) {
   
  -                throw new HttpException("Bad cookie header: illegal domain attribute \""
+ cookie.getDomain() + "\"");
  +                throw new HttpException("Illegal domain attribute \"" + cookie.getDomain()
+ "\"");
           }
       }
   
  
  
  
  1.80      +8 -7      jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java
  
  Index: HttpMethodBase.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v
  retrieving revision 1.79
  retrieving revision 1.80
  diff -u -r1.79 -r1.80
  --- HttpMethodBase.java	21 Nov 2002 09:54:52 -0000	1.79
  +++ HttpMethodBase.java	2 Dec 2002 06:14:56 -0000	1.80
  @@ -1446,9 +1446,10 @@
                                                   getPath(), conn.isSecure(), 
                                                   setCookieHeader);
                   state.addCookies(cookies);
  -            } catch (Exception e) {
  -                // FIXME: Shouldn't be catching exception!
  -                log.error("Exception processing response headers", e);
  +            } catch (HttpException e) {
  +                if (log.isWarnEnabled()) {
  +                    log.warn("Cookie rejected: \"" + setCookieHeader.getValue() + "\".
" + e.getMessage());
  +                }
               }
           }
       }
  
  
  

--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message