Closed
Bug 593170
Opened 14 years ago
Closed 14 years ago
see_also field incorrectly allows urls like "show_bug.cgi?id=2323" (with no domain)
Categories
(Bugzilla :: Creating/Changing Bugs, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.6
People
(Reporter: mkanat, Assigned: mkanat)
Details
Attachments
(1 file, 2 obsolete files)
1.26 KB,
patch
|
timello
:
review+
|
Details | Diff | Splinter Review |
The see_also field will let you input either of these without complaint:
show_bug.cgi?id=1
http://show_bug.cgi?id=1
When really we should be requiring full URLs (or now the individual numbers/aliases that we support on trunk).
Assignee | ||
Comment 1•14 years ago
|
||
Assignee: create-and-change → mkanat
Status: NEW → ASSIGNED
Attachment #471652 -
Flags: review?(timello)
Comment 2•14 years ago
|
||
Comment on attachment 471652 [details] [diff] [review]
v1
It accepts /show_bug.cgi?id=1 and as much as slashes we add in it and that would treat it as valid.
Also, I think we could just say that 'You must specify a full URL' instead, and then the reason name would be different to match with the message.
Attachment #471652 -
Flags: review?(timello) → review-
Assignee | ||
Comment 3•14 years ago
|
||
Good points on both of those! Fixed them both.
Attachment #471652 -
Attachment is obsolete: true
Attachment #471717 -
Flags: review?(timello)
Comment 4•14 years ago
|
||
Comment on attachment 471717 [details] [diff] [review]
v2
It is still accepting invalid URLs like http://////show_bug.cgi?id=1.
As you said, maybe we should do the check after the scheme check.
Attachment #471717 -
Flags: review?(timello) → review-
Assignee | ||
Comment 5•14 years ago
|
||
Okay, I wrestled with this a bit, and here's a check that I think covers everything! :-) Also, I wrote a much simpler comment. :-)
Attachment #471717 -
Attachment is obsolete: true
Attachment #471729 -
Flags: review?(timello)
Comment 6•14 years ago
|
||
Comment on attachment 471729 [details] [diff] [review]
v3
>=== modified file 'Bugzilla/Bug.pm'
>+ if (!$uri->authority or $uri->path !~ m{/}) {
>+ ThrowUserError('bug_url_invalid',
>+ { url => $input, reason => 'path_only' });
Nit: fix the indentation here.
And finally! It works!
Attachment #471729 -
Flags: review?(timello) → review+
Updated•14 years ago
|
Flags: approval?
Assignee | ||
Updated•14 years ago
|
Flags: approval? → approval+
Assignee | ||
Updated•14 years ago
|
Flags: approval4.0+
Flags: approval3.6+
Assignee | ||
Comment 7•14 years ago
|
||
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Bug.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7479.
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Bug.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7409.
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/Bug.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7174.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•