Closed Bug 1297243 Opened 4 years ago Closed 4 years ago

JSON bugzilla attachment reports charset='' (two apostrophes, which is an invalid parameter value) and can't be loaded.

Categories

(bugzilla.mozilla.org :: General, defect, P1)

Production
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: markh, Assigned: dylan)

References

()

Details

Attachments

(1 file)

In bug 1297234 there is an attachment https://bug1297234.bmoattachments.org/attachment.cgi?id=8783756 which fails to load - although the URL loads fine from cURL and is valid JSON.

The viewer shows "SyntaxError: JSON.parse: unexpected end of data at line 1 column 1 of the JSON data" and the console reports "Component returned failure code: 0x80500001 [nsIConverterInputStream.init]" pointing at converter-child.js
The exception happens since the charset isn't properly set.

@mayhemer: We are using the following to get the charset from the current channel:

this.charset = request.QueryInterface(Ci.nsIChannel).contentCharset || "UTF-8";

https://dxr.mozilla.org/mozilla-central/rev/f5d043ce6d36a3c461cbd829d4a4a38394b7c436/devtools/client/jsonview/converter-child.js#103

But it returns "''" (two apostrophes) 

Is this expected? Shouldn't `contentCharset` be just an empty string?

Honza
Flags: needinfo?(odvarko) → needinfo?(honzab.moz)
Examining |wget -S| I can see there is |Content-Type: application/json; charset=''| response header.  There from the two apostrophes come.

According rfc2822 and rfc2045 the charset part is a 'parameter'.  A parameter MAY be a quoted string which MUST be surrounded by DQUOTE (which is '"') only.

see https://tools.ietf.org/html/rfc2822#section-3.2.5

hence, this is a bugzilla bug.
Assignee: nobody → attach-and-request
Component: Developer Tools: JSON Viewer → Attachments & Requests
Flags: needinfo?(honzab.moz)
Product: Firefox → Bugzilla
QA Contact: default-qa
Summary: JSON bugzilla attachment can't be loaded. → JSON bugzilla attachment reports charset='' (two apostrophes, which is an invalid parameter value) and can't be loaded.
Assignee: attach-and-request → nobody
Component: Attachments & Requests → General
Priority: -- → P1
Product: Bugzilla → bugzilla.mozilla.org
QA Contact: default-qa
Version: unspecified → Production
While fixing Bugzilla is probably a good move, ISTM like we should be doing something better in this case.  Earlier versions of Firefox (48, for example) displayed it as plain text, for example, which is *much better* than what you get now. At the very least, the raw data should actually show up under the "Raw Data" tab.
Assignee: nobody → attach-and-request
Component: General → Attachments & Requests
Priority: P1 → --
Product: bugzilla.mozilla.org → Bugzilla
QA Contact: default-qa
Version: Production → unspecified
Assignee: attach-and-request → dylan
Component: Attachments & Requests → General
Priority: -- → P1
Product: Bugzilla → bugzilla.mozilla.org
QA Contact: default-qa
Version: unspecified → Production
The correct behavior is no charset= at all, surely?
Flags: needinfo?(honzab.moz)
Depends, if it's known to the server, then why not to provide it?  It's always better for a browser to know the encoding.  However, the charset parameter is purely optional.  But if that would be just '' then rather send nothing (because '' is no encoding).
Flags: needinfo?(honzab.moz)
See Also: → 365926, 1276820
Just an FYI, I added a charset to the attachment in bug 1297234 and it loads. Working on a proper fix which will be out next Tuesday.
Flags: needinfo?(markh)
1) I think we can detect encoding on application/json in addition to text/* types.
2) We need to do something else when we can't detect an a charset. Testing out options now.
Attached patch 1297243_1.patchSplinter Review
I see no evidence that apache ever replaces $cgi->charset('') with anything,
so this should work. On my testing machine it results in a content type with no charset. I'm testing this on dev too: https://bug1153636.bugzilla-dev.allizom.org/attachment.cgi?id=8591901

There will be a second patch to do charset detection for JSON files.
Attachment #8792026 - Flags: review?(dkl)
SGTM - this isn't blocking me in any way.
Flags: needinfo?(markh)
Comment on attachment 8792026 [details] [diff] [review]
1297243_1.patch

Review of attachment 8792026 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Attachment #8792026 - Flags: review?(dkl) → review+
To git@github.com:mozilla-bteam/bmo.git
   381c4dd..e1dc4f6  master -> master
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.