Bug 132344

Summary: [patch] www/en/cgi/query-pr.cgi broken base64 attachments
Product: Documentation Reporter: David Horn <dhorn2000>
Component: Books & ArticlesAssignee: Gavin Atkinson <gavin>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff
none
query_pr.patch.txt none

Description David Horn 2009-03-05 21:10:01 UTC 
The query-pr.cgi script interface to gnats tries very hard to decode and display attachments properly.

www/en/cgi/query-pr.cgi

Unfortunately, MIME headers do not always play nice, and there are many instances of inproper display of base64 encoded attachments.

The real issue is that if either "X-Attachment-Id:" or "Content-Disposition:" headers occur after "Content-Transfer-Encoding" headers, then decode_base64() fails, and displays unprintable characters.

example pr's that do not display base64 attachments properly:

ports/130991
ports/130144
ports/130000

Fix: -Parse out the X-Attachment-Id header if found
-do not assume that content-transfer-encoding is the last header
-Check for nonprintables on result of decode_base64()

This fix seems to work for all broken base64 attachment pr's that I have found.  To test the check for nonprintables, just mangle one of the prs by adding "X-Broken-Header: asdfasdf" to the end of the mime header just before the base64 payload starts, and the code will break again, but at least will be caught by the :isascii: check for nonprintables

Unified diff attached.

Patch attached with submission follows:
How-To-Repeat: Go to http://www.freebsd.org/cgi/query-pr.cgi?pr=130144 in your web browser, and notice that the attachment displays unprintable characters.

Repeat for 130144 and 130000 as slightly different examples.
Comment 1 David Horn 2009-03-05 21:27:35 UTC 
Some of my example pr numbers are a bit off (I guess my typing is not
what it used to be)

Examples should have been:

131991
130144
130000

and now for completeness, this pr will get an attachment as well so
132344 will fail to display the attachment until this fix is in place,
as the problem always occurs from gmail (at least for me).

--Thanks!

--_Dave Horn

Non quia difficilia sunt non audemus, sed quia non audemus, difficilia sunt.
Comment 2 Mark Linimon freebsd_committer freebsd_triage 2009-03-09 17:28:08 UTC 
Responsible Changed
From-To: freebsd-www->bugmeister

bugmeister territory.  (I am already looking at a patch for this).
Comment 3 Gavin Atkinson freebsd_committer freebsd_triage 2009-11-15 17:40:33 UTC 
I think http://people.freebsd.org/~gavin/PRs/132344.diff is a better fix.

A ":" is not valid within MIME or BASE64 encoded data, so the added lines 
are a safe way of stripping out any extra header lines once we've found 
the "Content-Transfer-Encoding:" header.  This patch has the advantage of 
handling all headers, rather than just the two we know about so far. 
There is in theory no reason why we won't see a "X-Foo-Bar:" header in the 
future, so no sense in limiting what we strip out when we can already know 
if it is actually a header.

THis is a minimal patch to fix the issue, the real fix would involve 
refactoring the code for no real benefit.

Gavin
Comment 4 dfilter service freebsd_committer freebsd_triage 2009-11-15 18:40:44 UTC 
remko       2009-11-15 18:40:26 UTC

  FreeBSD doc repository

  Modified files:
    en/cgi               query-pr.cgi 
  Log:
  Correct display of BASE64 attachements.
  
  PR:             116594 [1], 132344 [2]
  Submitted by:   edwin [1], David Horn [2]
  Patch from:     gavin
  
  Revision  Changes    Path
  1.69      +3 -2      www/en/cgi/query-pr.cgi
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 5 Gavin Atkinson freebsd_committer freebsd_triage 2009-11-15 21:24:11 UTC 
State Changed
From-To: open->closed

A different patch has been committed, but based entirely on your 
analysis of the problem.  Many thanks!
Comment 6 Gavin Atkinson freebsd_committer freebsd_triage 2009-11-15 21:24:11 UTC 
Responsible Changed
From-To: bugmeister->gavin