launchpad-reviewers team mailing list archive

[İlker Emre Koç <mp+481785@xxxxxxxxxxxxxxxxxx>]

The proposal to merge ~ilkeremrekoc/launchpad:accept-crlf-bugreports into launchpad:master has been updated.

Description changed to:

FileBugDataParser is the only parser that works on ApportJobs i.e. jobs from Apport package. Originally, only the LF line-break was expected from Apport, and a hard-coded logic was put on it within FileBugDataParser. The problem is, this was in violation of the RFC standards for HTTP request formats, which only accepts CRLF as its linebreaks. Because of this, an upgrade to multipart package which parsed multipart/form-data requests broke the apport reporting system by becoming more strict in its parsing by refusing to parse LF line-endings.

Long story short, to upgrade multipart we need to patch apport package to only send CRLF line-breaks, but to patch apport package we need to make FileBugDataParser accept CRLF line-breaks in the first place.

This commit is intended to make FileBugDataParser accept CRLF while retaining LFs for backward compatibility.

Some of the things I needed to do to ensure this is in need of discussion though. Mainly, the way I implemented the "reset" to the input blob stream is less-than-perfect as I only had to do that since the Librarian-File-Alias class which FileBugDataParser uses doesn't have a "seek" functionality, requiring a "close()" -> "open()" call within the method instead.

Another discussion point could be the inheritance of BytesIO as MockBytesIO inside one of the testing modules, mainly because BytesIO doesn't have any "open()" method but has a "close()" method that act differently than the ones in file-alias, requiring a simple implementation of a child class for BytesIO that implemented an "open()" method.

I also added a new test text-file into the codebase to ensure we have a message file that uses CRLF for its line-breaks so that we can test the ApportJobs better.

For more details, see:
https://code.launchpad.net/~ilkeremrekoc/launchpad/+git/launchpad/+merge/481785
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilkeremrekoc/launchpad:accept-crlf-bugreports into launchpad:master.