CGI.pl warning: Character in "c" format wrapped...

RESOLVED FIXED in Bugzilla 2.16

Status

()

--
major
RESOLVED FIXED
16 years ago
6 years ago

People

(Reporter: peter.pundy, Assigned: bbaetz)

Tracking

unspecified
Bugzilla 2.16
x86
Linux
Bug Flags:
approval +

Details

(Whiteboard: [fixed in 2.16.3])

Attachments

(1 attachment)

(Reporter)

Description

16 years ago
User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)
Build Identifier: 

My bugzilla server just had its disk filled with apache error log containing 
this message:

[Wed Feb 19 17:09:16 2003] [error] [client 192.168.0.44] [Wed Feb 19 17:09:16 
2003] attachment.cgi: Character in "c" format wrapped at CGI.pl line 83., 
referer: http://bugzilla.optiwave.com/dev/attachment.cgi


Reproducible: Didn't try

Steps to Reproduce:
Sorry, haven't got a non-production platform to attempt reproduction yet.
Actual Results:  
No reproduction results yet, sorry.

Expected Results:  
CGI.pl / attachment.cgi shouldn't produce errors so frequently as to fill a 1G 
partition.

Line 83 code of CGI.pl is:

$todecode =~ s/%([0-9a-fA-F]{2})/pack("c",hex($1))/ge;

I could be wrong, but hex() returns an unsigned value 0-255 yes?  Which should 
be matched with a "C" format, not a "c" format.

That's the patch I'm testing now.  (I'm using v2.16.2)
(Assignee)

Comment 1

16 years ago
Hmm. This should only be an issue with non-ascii names, and we don't really
support that. Is that the case for you? What perl version are you using?

Current CGI.pm (which CVS-HEAD bugzilla uses) uses just uses |chr| instead of
pack. We should probably do that too.
Whiteboard: [wanted for 2.16.3]?
Target Milestone: --- → Bugzilla 2.16
(Reporter)

Comment 2

16 years ago
Some additional facts:
- we use perl v5.8.0
- there were over 2 million lines in the log file
- only a couple hundred did not contain 'Character "c" format wrapped
- the last 1000 lines were all the error message and occurred over the space of 
2.5 minutes

Now, do you mean non-ascii filenames, user names?  I did change the system 
locale semi-recently from en_US.utf8 to en_US.iso885915

My wonder is what operation would pass over that CGI.pl line enough times to 
generate a couple million warnings in an hour or so?
Strange, I just had this happen on a test install when I tested a relatively
simple patch (bug 193947), although in that case it might have been my fault (I
may have introduced an infinite loop).  Still, one has to wonder if the error
trapping code is at fault.
(Assignee)

Comment 4

16 years ago
We don't have any eror trapping code, really.

I reproduced this errro using |http://mango.home/bugzilla-2.16/?foo=%ff|. I
wonder if an attachment somehow went through url_decode. It shouldn't have.
anywya, heres not much point in looking into this too deeply, given that HEAD
fdoesn't use that code.
(Reporter)

Comment 5

16 years ago
Myk/Bradley: when you say reproduced, do you mean you achieved in getting a 
single error message, or a significant number of them?  I, too, wondered if 
attachment data was somehow getting fed through url_decode.  

While it may not be worth investigating too much farther, if anyone has 
familiarity with the code area, it would be worthwhile to:

- comment on a patch for live production 2.16.2 versions consisting of chaning 
CGI.pl:83 pack("c" to pack("C"

- confirm that the HEAD version doesn't suffer from the same weakness even 
though it is coded differently (do the apis used in HEAD avoid this data 
conversion error?, is attachment data be processed by url_decode()?, is that 
supposed to happen?)
(Assignee)

Comment 6

16 years ago
I only saw the one error per character. This does not happen with a HEAD tree.

Attachments shouldn't be being sent via a url, so url_decode shouldn't be being
called. If you can track down a case where atachment data is ending up that way,
then tahts a separate issue (which may or may not be a bug, depending)

Instead of using pack, just use |chr|.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Anyone have a patch for this yet?  We're going to have a 2.16.3 shortly, if we
want this included in it....
(Assignee)

Comment 8

16 years ago
Created attachment 120422 [details] [diff] [review]
fix

OK, trivial fix attached. %ff in url params warns w/o this patch, but not with
it.
(Assignee)

Comment 9

16 years ago
-> me
Assignee: myk → bbaetz
(Assignee)

Updated

16 years ago
Attachment #120422 - Flags: review?(justdave)
Attachment #120422 - Flags: review?(justdave) → review+
approved for 2.16 branch
Flags: approval+
Whiteboard: [wanted for 2.16.3]? → [wanted for 2.16.3]
(Assignee)

Comment 11

16 years ago
Fixed-on-branch; not needed on trunk
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Summary: CGI.pl produces apache error_log: Character in "c" format wrapped... → CGI.pl warning: Character in "c" format wrapped...
Whiteboard: [wanted for 2.16.3] → [fixed in 2.16.3]
*** Bug 219567 has been marked as a duplicate of this bug. ***
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.