adding invalid addresses to CC list results in html error displayed as text

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Creating/Changing Bugs
P3
blocker
RESOLVED FIXED
16 years ago
6 years ago

People

(Reporter: John Vandenberg, Assigned: myk)

Tracking

({regression})

2.15
Bugzilla 2.16
regression

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

16 years ago
When entering an invalid email address in either the Assigned To or CC input
boxes, the error message returned in DBNameToIdAndCheck is displayed without a
Content-type header preceeding it.
(Reporter)

Comment 1

16 years ago
Within post_bug.cgi, calls to CheckFormField and CheckFormFieldDefined which
result in an error, and the two direct calls to PuntTryAgain, all result in Bad
Header errors.

Steps to reproduce:
1) Simulate a enter_bug.cgi page, and remove the hidden product field, or set
the product field to a value which is not valid.
2) Submit form

Expected Results:
Error messages indicating that a product field must be defined, and must have a
valid value.

Actual Results:
500 Internal Server Error
Severity: critical → normal
Summary: adding invalid addresses to CC list results in html error displayed as text → error messages display incorrectly or cause Internal Server Error
Severity: normal → blocker
Keywords: regression
Priority: -- → P3
Summary: error messages display incorrectly or cause Internal Server Error → adding invalid addresses to CC list results in html error displayed as text
Target Milestone: --- → Bugzilla 2.16
John, this sounds like two separate issues, can you file a separate bug for the
second one?  Thanks.

Making a 2.16 blocker, entering a bad CC address is a pretty common thing to do,
the error messages for it need to work right.
(Reporter)

Comment 3

16 years ago
As far as I can see this is the same basic problem as bug 109054.

The simple fix for this is to move the Content-type up higher to be just
underneath the last DisplayError.  The 500's and plain text then disappear and a
page without a Bugzilla header results.  A header can not be displayed at this
point as the title is not known until later, as it displays the newly created
bug number in the title.

A better solution would be to templatise DBNameToIdAndCheck() and resolve bug
136770 at the same time.
Where we are heading with Content-Types is that they should get printed just
before templates; all error messages up to that point should use DisplayError(),
which prints a Content-Type of its own.

This presents a slight problem, because the admin CGIs, which are not yet
templatised, may get double content-types. But admins can live with that :-)

So, change the calls from PuntTryAgain to ThrowUserError, with the
$unlock_tables parameter set to true.

Gerv
(Reporter)

Comment 5

16 years ago
In this case, DBNameToIdAndCheck is in globals.pl , whereas ThrowUserError and
friends are in CGI.pl  
The 500 errors due to PuntTryAgain are being addressed in bug 138879, however I
will try and get a patch up for it based on your suggestion.
> In this case, DBNameToIdAndCheck is in globals.pl , whereas ThrowUserError and
> friends are in CGI.pl 

Oops. Does this mean that we can't use them in globals.pl? If so, that's a bit
bad - we'll need to move them to globals.pl. We want to be able to use them
everywhere.

Gerv
Created attachment 80844 [details] [diff] [review]
Patch v.1

Puts correct error handling into DBNameToIDAndCheck, and fixes bug 136770 along
the way.

Gerv
Created attachment 80847 [details] [diff] [review]
Patch v.2

<cough> This one doesn't have Mail stuff mixed up in it...

Gerv
Attachment #80844 - Attachment is obsolete: true
(Assignee)

Comment 9

16 years ago
Comment on attachment 80847 [details] [diff] [review]
Patch v.2

works, trivial, 2xr=myk
Attachment #80847 - Flags: review+
Fixed.

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.158; previous revision: 1.157
done

Gerv
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.