Closed
Bug 214466
Opened 21 years ago
Closed 21 years ago
Cookie setting w/ 3xx redirect is broken on IIS (colchange)
Categories
(Bugzilla :: Query/Bug List, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: cbendell, Assigned: jouni)
Details
(Whiteboard: [needed for Win32bz])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
justdave
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624
Changing the columns using the "change columns" link at the bottom of the result
list does not change the columns displayed. I have tested this in IE6 and
Mozilla 1.5
Reproducible: Always
Steps to Reproduce:
1. performed query
2. selected 'change columns'
3. selected different columns and submitted the page
4. results display the original columns, and not the new column list
5. 'change columns' displays only the original column selections, not the new
selection
Reporter | ||
Comment 1•21 years ago
|
||
Forgot to mention that this bug is reproduceable with the latest cvs checkout of
bugzilla.
Comment 2•21 years ago
|
||
I could not reproduce this in my bugzilla or in
http://landfill.bugzilla.org/bugzilla-tip/
Do you have cookiepath parameter set up correctly? Does the value of COLUMNLIST
cookie change for you when you change the columns?
Reporter | ||
Comment 3•21 years ago
|
||
The problem is that the Set-Cookie directives are not coming through with the
304 redirect directive. If I disable the redirect, forcing the page to return
200 OK, the cookies do get set and everything behaves from there on in as
expected.
The only difference I can see from out configuration and landfill.bugzilla.org
is that we are fronting bugzilla with IIS (5.0). I have confirmed that IIS is
not mangling the 304 with its own error page. I am going to attempt to revert
to the 1.34 method of outputting the header information (instead of using the
cgi pm)
Reporter | ||
Comment 4•21 years ago
|
||
I'm no closer to understanding why the Set-Cookie headers aren't propagated to
the user, but if I replace the $cgi->redirect method with a refresh header (
print "Refresh: 0; URL=buglist.cgi?$::FORM{'rememberedquery'}\n"; ) then
everything works as I would expect because a 200 OK is sent to the user and
the browser follows the redirect.
Is there a test environment that has IIS fronting Bugzilla instead of Apache?
(I'm using IIS only because I want Domain authentication for Bugzilla and View
CVS)
Comment 5•21 years ago
|
||
I do not have any experience with IIS, but if Jouni doesn't mind, I think he
might be able to help.
Assignee | ||
Comment 6•21 years ago
|
||
There's no public IIS based Bugzilla test installation. I've had IIS test
platforms for quite some time, but they're never stable enough to be allowed
public use ;-)
Your problem is called an IIS bug. I had all forgotten about this, but it's one
part in the huge mess about how IIS handles the details of NPH CGIs.
http://support.microsoft.com:80/support/kb/articles/q176/1/13.asp&NoWebContent=1
The workaround is not practically usable in Bugzilla.
See bug 201816 for what caused this. This will break column changing when
updating Bugzilla on all Windows/IIS installations (apart from Windows 2003
server with IIS 6, which is rumored to work correctly).
Bbaetz, do you have a proposal on how to fix this? Give up on NPH mode?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: "change columns" does not change columns → nph redirect on column change breaks on IIS
Whiteboard: [needed for Win32bz]
Comment 7•21 years ago
|
||
I thought that IE didn't support Refresh - thats one of the reasons it got moved.
Note that NPH mode won't work either - see
http://support.microsoft.com/default.aspx?scid=kb;en-us;280341
Now that we use CGI.pm for everything, we could run in non-parsed header mode, I
guess - can we do that w/o the nph- prefix? In CGI.pm, theres a commented out line:
# $NPH++ if defined($ENV{'SERVER_SOFTWARE'}) && $ENV{'SERVER_SOFTWARE'}=~/IIS/;
if you uncomment that, does that fix things, and does it break anything else?
(buglists on mozilla/ns4 being the main one which may break)
Reporter | ||
Comment 8•21 years ago
|
||
This is more a philosophical thought: What about setting support for bugzilla
to IIS 5 and greater? IIS 5 is available for NT4 and that kb #280341 only
relates to IIS 4.
Presumeably all these problems are resolved with IIS 6 since it was a complete
rewrite, but that just might me being optimistic.
Comment 9•21 years ago
|
||
Re: comment #4: for the record, you *can* do domain authentication with Apache.
Look for mod_sspi
Reporter | ||
Comment 10•21 years ago
|
||
Responding to #4:
IE (IE 5.0+) _does_ support the refresh. Perhaps an old version (-4.0) didn't?
I couldn't find the line in CGI.pm you are suggesting to comment out. This is
in Bugzilla::CGI.pm right?
Comment 11•21 years ago
|
||
I ran into this IIS header bug on an internal project at work before. It was the
last bit of ammunition I needed to get them to let me put Apache on that box.
Unfortunately, there are some things that require IIS so if you use any of them
ont that box switching is not really an option (OWA on Exchange server leaps to
mind).
Comment 12•21 years ago
|
||
No, I meant the perl CGI.pm module.
Assignee | ||
Comment 13•21 years ago
|
||
Re comment 8, Colin:
Requiring IIS 5 would be fine with me if it helped, but it doesn't. The KB
article you refer to is a separate problem. Even though that one is fixed in IIS
5, the nph handling still has problems. It works (at least theoretically) if you
rename a cgi to "nph-something.cgi", but there's quite a lot of work to be done
if one wishes that to be made for all the pages that use server side push...
Jake, you're right in hinting that using Apache with this kind of software is
massively easier. However, there is the point of not having to maintain two
different web servers even if you're not absolutely forced to (by OWA for
example). Of course, all this is relevant only if you're running production IISs
anyway.
Updated•21 years ago
|
Target Milestone: --- → Bugzilla 2.18
Assignee | ||
Comment 14•21 years ago
|
||
Taking the bug, and ccing some core posse for comments. To summarize the previous:
IIS functions incorrectly when trying to set cookies simultaneously with a 3xx
redirect. This would be fixed by using nph mode for the cgi, but for IIS, that
apparently requires renaming the cgi to be "nph-colchange.cgi". That's pretty
much impossible, so let's forget it.
Bbaetz's proposal of hacking cgi.pm in comment 7 didn't help, and toying with
IIS seems to be very difficult. I think we have two major alternatives:
1) make IIS users patch this by hand, essentially making this wontfix (but this
is somewhat contrary to the roadmap's "out of the box" target)
2) introduce a fallback iis compatibility hack so that instead of using an HTTP
3xx redirect, we use the old-fashioned Refresh header
As for number 2: I don't know if we can automagically detect the web server
we're running under. This would mean making people set "IIS mode" on somewhere.
That's ugly, but maybe acceptable.
If we decide to create this sort of "compatibility mode", how should we switch
it on? I'm thinking of a Win32-only localconfig switch or something of equal
caliber. Would you find this acceptable? Better alternatives?
If we use refresh headers for IIS only, do you see other problems involved?
Assignee: justdave → jouni
Summary: nph redirect on column change breaks on IIS → Cookie setting w/ 3xx redirect is broken on IIS (colchange)
Comment 15•21 years ago
|
||
I thought the reason we got rid of the Refresh: header is that Internet Explorer
didn't support it...
Assignee | ||
Comment 16•21 years ago
|
||
Yeah, but see comment 10 on that. I haven't confirmed it though, but I've never
encountered the Refresh problem with IE myself, so I'd think it's a pre-5.0
problem if something.
Comment 17•21 years ago
|
||
Comment 6 seems to indicate that this bug is fixed in IIS6. Is that correct?
If that's the case, then I have no problem with providing the Refresh: header or
the even older <meta name="refresh"> foo (whichever IE is more likely to
support) with the setting of a param or a variable in localconfig or something.
refreshhack Older versions (prior to version 6) of Microsoft Internet
Information Server (IIS) had a bug which prevented them from
properly setting cookies at the same time as issuing a 3xx
redirect. If you are using a version of IIS older than version 6,
you will need to turn this on to make changing columns in
buglists work.
( ) On (*) Off
Something along those lines...
Comment 18•21 years ago
|
||
you can look at ENV{SERVER_VERSION}. Can we do this in the CGI.pm redirect
function? - ie convert the 30x to a 200 + refresh if we're IIS?
Comment 19•21 years ago
|
||
It would be much better if this didn't have to be a param - and, from what
bbaetz says, it probably doesn't - which is good.
Gerv
Assignee | ||
Comment 20•21 years ago
|
||
SERVER_SOFTWARE, actually. See <http://hoohoo.ncsa.uiuc.edu/cgi/env.html>. But
yeah, that looks like a feasible approach.
Status: NEW → ASSIGNED
Assignee | ||
Comment 21•21 years ago
|
||
Ok, this is try 1.
- Instead of changing Bugzilla::CGI->redirect, I hacked colchange. This is
because we use quite a few redirects in Bugzilla altogether, and this IIS
hack only concerns those places where cookies are set.
- This also uses the old mechanism on IIS6. I don't have it available for
testing, so I can't confirm if the correct approach works or not. I don't
think IIS 6 is an issue anyway, and I suggest we fix (optimize) it when we
can properly test this.
- I tried the now-revived refresh header on a couple of IEs (5.something and
6.0) and it worked fine on both. This may have a problem on older IEs, but
I don't know if it's a problem. I guess we'll hear if somebody hits a
problem. Writing out meta refresh tags would complicate the solution
somewhat, so I don't think it's worth doing if it's only IE 3 that suffers
from this.
Assignee | ||
Updated•21 years ago
|
Attachment #135259 -
Flags: review?(bbaetz)
Assignee | ||
Comment 22•21 years ago
|
||
Oh, forgot to say: Tested on Windows 2k / IIS 5.0 and RH 8 / Apache 1.3.27. Of
course, the patch shouldn't affect platforms other than IIS.
Reporter | ||
Comment 23•21 years ago
|
||
You shouldn't have worry about supporting IE4 or IE3 since they make up less
than 1% of the browser market.
http://www.w3schools.com/browsers/browsers_stats.asp
http://www.webreference.com/stats/browser.html
Comment 24•21 years ago
|
||
from 'perldoc CGI' :
GENERATING A REDIRECTION HEADER
print $query->redirect('http://somewhere.else/in/movie/land');
Sometimes you don't want to produce a document yourself, but simply
redirect the browser elsewhere, perhaps choosing a URL based on the
time of day or the identity of the user.
The redirect() function redirects the browser to a different URL. If
you use redirection like this, you should not print out a header as
well.
One hint I can offer is that relative links may not work correctly when
you generate a redirection to another document on your site. This is
due to a well-intentioned optimization that some servers use. The
solution to this is to use the full URL (including the http: part) of
the document you are redirecting to.
You can also use named arguments:
print $query->redirect(-uri=>'http://somewhere.else/in/movie/land',
-nph=>1);
The -nph parameter, if set to a true value, will issue the correct
headers to work with a NPH (no-parse-header) script. This is important
to use with certain servers, such as Microsoft IIS, which expect all
their scripts to be NPH.
Notice the comment about IIS at the end.
Assignee | ||
Comment 25•21 years ago
|
||
Comment on attachment 135259 [details] [diff] [review]
v1
Cancelling review request as bug 223473 bitrotted the patch anyway. Also to
re-investigate the nph stuff pasted by justdave; I've got some ideas.
Attachment #135259 -
Flags: review?(bbaetz)
Assignee | ||
Comment 26•21 years ago
|
||
As much as I hate to admit it, I haven't been able to come up with a better
fix. What Dave pasted is fine, except for one fact: It somehow assumes IIS
really treats all cgis as NPH. But in fact IIS only treats CGIs in nph _if_
their names are prefixed with "nph-". Oddly enough, even when I created
nph-colchange.cgi to handle the submit, it didn't work. Well, this is, of
course, a sum of several issues. The root cause is the fact that IIS's CGI
handling simply sucks when it comes to stuff like this.
I suggest we proceed with this sort of a patch unless somebody else can help
with this. I can't make it break on anything, and it's not like I was close to
a breakthrough that would make the fix cleaner or meaner... If I come up with a
better solution later on, I'll be sure to contribute it.
Assignee | ||
Updated•21 years ago
|
Attachment #135259 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #135831 -
Flags: review?(justdave)
Comment 27•21 years ago
|
||
Comment on attachment 135831 [details] [diff] [review]
v2
we can't print headers to stdout anymore. Use $cgi->header().
print $cgi->header(-type => "text/html",
-refresh => "0; URL=$vars->{'redirect_url'});
Something to that effect (double-check the docs for the correct syntax)
Attachment #135831 -
Flags: review?(justdave) → review-
Comment 28•21 years ago
|
||
I'm missing a quote there if you cut/paste
Assignee | ||
Comment 29•21 years ago
|
||
Docs checked and tested to work on IIS5/W2k.
Attachment #135831 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #135835 -
Flags: review?(justdave)
Updated•21 years ago
|
Attachment #135835 -
Flags: review?(justdave) → review+
Updated•21 years ago
|
Flags: approval?
Updated•21 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 30•21 years ago
|
||
Checking in colchange.cgi;
/cvsroot/mozilla/webtools/bugzilla/colchange.cgi,v <-- colchange.cgi
new revision: 1.38; previous revision: 1.37
done
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 31•19 years ago
|
||
Hope it doesn't mess anything up responding to an old bug like this, but I spent the day wrestling with colchange.cgi on a newly-installed 2.2 site and ended up finding this bug...
Short synopsis: when I would make a few changes to the list of columns and submit it, the server would return a "404 Not Found" error on colchange.cgi. If I forced the script to output a HTTP header up top then colchange.cgi worked and I could see the cookies it was trying to set, and it would display the "next" page, but the cookies obviously weren't getting set because they were printing to the output page. Any time the debug header was removed the 404 error returned.
Anyway, after finding this bug I found the lines in the code where you change the cookie-setting depending on what SERVER_SOFTWARE is being used. On a hunch I commented out the ifs and forced it to use the alternative code, and it worked fine from that point.
My SERVER_SOFTWARE in this case is "iPlanet-WebServer-Enterprise/4.1".
I don't know if you want to add that to the if statement going forward or what, but consider this another data point on this bug (if I should open this as a New bug instead please let me know).
Thanks.
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•