tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Hanik - Dev Lists <devli...@hanik.com>
Subject Re: svn commit: r786667 - /tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
Date Mon, 22 Jun 2009 15:14:29 GMT
Mark Thomas wrote:
> Filip Hanik - Dev Lists wrote:
>   
>> we're copying bytes to two new byte arrays on every call, just so that
>> we have info when it fails?
>>     
>
> For query string there is (and always has been) one copy and then this
> patch copies again for the failure info.
>
> For POST requests the only copy is for the failure info.
>
>   
>> especially only logged with debugEnabled, shouldn't your logic check
>> that flag then
>>     
>
> The debug logging includes the exception. There is still warn level
> logging that provides the information on the failed parameters.
>
> One option to reduce the copying would be to log the failure with the
> corrupted data at warn level along with the info that if you use debug
> logging you'll see the original data. That would move the copying to
> debug only although you might see some odd messages depending on how the
> decoding failed.
>
> What do you think?
>   
That sounds good to me. One could adjust the warn message to let the 
user know that more info is available during debug.

Filip
> Mark
>
>   
>> Filip
>>
>>
>> markt@apache.org wrote:
>>     
>>> Author: markt
>>> Date: Fri Jun 19 21:14:32 2009
>>> New Revision: 786667
>>>
>>> URL: http://svn.apache.org/viewvc?rev=786667&view=rev
>>> Log:
>>> Can't use queryMB as that isn't the only source.
>>>
>>> Modified:
>>>     tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
>>>
>>> Modified: tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java?rev=786667&r1=786666&r2=786667&view=diff
>>>
>>> ==============================================================================
>>>
>>> --- tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java
>>> (original)
>>> +++ tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java Fri
>>> Jun 19 21:14:32 2009
>>> @@ -338,6 +338,8 @@
>>>      // if needed
>>>      ByteChunk tmpName=new ByteChunk();
>>>      ByteChunk tmpValue=new ByteChunk();
>>> +    ByteChunk origName=new ByteChunk();
>>> +    ByteChunk origValue=new ByteChunk();
>>>      CharChunk tmpNameC=new CharChunk(1024);
>>>      CharChunk tmpValueC=new CharChunk(1024);
>>>      @@ -396,18 +398,25 @@
>>>              }
>>>              tmpName.setBytes( bytes, nameStart, nameEnd-nameStart );
>>>              tmpValue.setBytes( bytes, valStart, valEnd-valStart );
>>> -
>>> +            try {
>>> +                // Take copies as if anything goes wrong originals
>>> will be
>>> +                // corrupted. This means original values can be logged
>>> +                origName.append(bytes, nameStart, nameEnd-nameStart);
>>> +                origValue.append(bytes, valStart, valEnd-valStart);
>>> +            } catch (IOException ioe) {
>>> +                // Should never happen...
>>> +                log.error("Error copying parameters", ioe);
>>> +            }
>>> +                         try {
>>>                  addParam( urlDecode(tmpName, enc),
>>> urlDecode(tmpValue, enc) );
>>>              } catch (IOException e) {
>>> -                // tmpName or tmpValue will be corrupted at this
>>> point due to
>>> -                // failed decoding. Have to go to queryMB to get
>>> original data
>>>                  StringBuilder msg =
>>>                      new StringBuilder("Parameters: Character decoding
>>> failed.");
>>>                  msg.append(" Parameter '");
>>> -                msg.append(queryMB.toString().substring(nameStart,
>>> nameEnd));
>>> +                msg.append(origName.toString());
>>>                  msg.append("' with value '");
>>> -                msg.append(queryMB.toString().substring(valStart,
>>> valEnd));
>>> +                msg.append(origValue.toString());
>>>                  msg.append("' has been ignored.");
>>>                  if (log.isDebugEnabled()) {
>>>                      log.debug(msg, e);
>>> @@ -418,7 +427,8 @@
>>>  
>>>              tmpName.recycle();
>>>              tmpValue.recycle();
>>> -
>>> +            origName.recycle();
>>> +            origValue.recycle();
>>>          } while( pos<end );
>>>      }
>>>  
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>
>>>
>>>   
>>>       
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>     
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>   


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


Mime
View raw message