Closed Bug 138456 Opened 22 years ago Closed 22 years ago

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

Categories

(Bugzilla :: Creating/Changing Bugs, defect, P3)

2.15
defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: jayvdb, Assigned: myk)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
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.
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
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
Attached patch Patch v.1 (obsolete) — Splinter Review
Puts correct error handling into DBNameToIDAndCheck, and fixes bug 136770 along
the way.

Gerv
Attached patch Patch v.2Splinter Review
<cough> This one doesn't have Mail stuff mixed up in it...

Gerv
Attachment #80844 - Attachment is obsolete: true
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
Closed: 22 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.

Attachment

General

Created:
Updated:
Size: