800196 - Newlines in multi-line fields issue

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Simon Green]

(Originally reported in bugzilla.redhat.com):

Description of problem:
Bugzilla provides custom fields that allow entering multi-line text. However, it does not seem to enforce what endlines should be used in those fields (\n vs. \r\n), which results in confusing endline-only changes when using both xmlrpc and web ui, as well as xmlrpc clients getting unexpected endlines.

Steps to Reproduce:
1. Set multi-line text to one of the fields, such as "line1\nline2\nline3\n"
2. Open the bug in webui and do some change (add comment, change state, add cc, ...) without changing that multi-line field.  This can be done on Linux using firefox, does not need to be done on the platform that uses \r\n natively.
3. Check history of the bug, see it notes field change, though you can't really figure out what the change was from the webui.
4. Fetch field value using xmlrpc.  See that it now contains unexpected \r.
  
Expected results:
BZ should one endline type internally, and convert endlines on input/output as needed.  E.g. if \n is to be used internally, BZ should do \r\n -> \n on the data it receives from the web browsers to avoid confusing whitespace-only changes.  If \r\n is chosen to be used internally, BZ should do \n -> \r\n on xmlrpc inputs and \r\n -> \n on xmlrpc outputs.  Working around in xmlrpc clients is possible, but sounds like a wrong place to fix this.

Comment 1

[Simon Green]

Before I go and submit a patch my question is do we fix this so all XMLRPC text fields submit consistent newlines (i.e. \r\n), or do we change it if the only change to a field is to the new line characters, we ignore that change?

I'm thinking the first option is preferred, since it keeps everything consistent.

And if it is the first option, where is the best place to do it? Bugzilla::Object's set would ensure everything has a new line, but over-ridding  set in Bugzilla::Bug might be a safer option.

I'm hesitant to change this in the RPC calls themselves, since other forms of bug submittal (e.g. email_in.pl might also have this issue)

  -- simon

Comment 2

[Frédéric Buclin]

How is that different from comments you get via XML-RPC? Comments also contain newlines.

Comment 3

[Simon Green]

(In reply to Frédéric Buclin from comment #2)
> How is that different from comments you get via XML-RPC? Comments also
> contain newlines.

Comments aren't edited (well they can be in brc, but not upstream). The problem we are having is that a multi text custom field that is set by XMLRPC has \n as the new line character. If someone edits the bug, the \n is changed to \r\n which then marks this field as changed, when it really hasn't been.

From what I can see (read: Google), HTTP specifications say that newlines should be sent as \r\n, so it's not something that we can fix browser side.

My original questions is what to upstream think is the best way to solve the issue?

  -- simon

Comment 4

[Frédéric Buclin]

I would say, use \n and convert \r\n to \n on input. This must be done in the _check_multi_select_field() validator in Bugzilla::Bug.

Comment 5

[Matt Tyson]

Attachment: sanitise \n to \r\n for FIELD_TYPE_TEXTAREA

This patch will sanitise \n line endings to \r\n in multi line custom text fields (FIELD_TYPE_TEXTAREA).

I think this is the best approach as browsers will submit line endings as \r\n and that's what will be in the database already.  The problem tends to happen when XMLRPC clients submit \n line endings.

Comment 6

[:glob ✱]

Comment on attachment 678202 [details] [diff] [review]
sanitise \n to \r\n for FIELD_TYPE_TEXTAREA

r=glob
i agree that using \r\n internally is the better approach.

Comment 7

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Bug.pm
Committed revision 8464.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/Bug.pm
Committed revision 8449.