If you ask GetServerVariable() for multiple header fields by specifying "ALL_HTTP" or "ALL_RAW" as variable_name, each field is terminated with LF. This differs from the behavior of IIS, which returns fields compliant with RFC2616 (CRLF terminators). For once I'd argue that IIS got it right and is in compliance with the spec.
Created attachment 6742 [details] Trivial patch
enabling the PatchAvailable keyword updated doc on submitting patches is at http://httpd.apache.org/dev/patches.html
I'd like to see this patch (and those for 20617 and 20619) applied. All three patches are simple, and I think they're correct and fix errors in the mod_isapi implementation. Is there anything I can do to facilitate this? Should I: - further explain why I believe mod_isapi is currently broken? - bring the patches to someone else's attention? - add votes or keywords? - change priority or severity? - prepare the patch differently? (I think I got it right.) - provide test code that demonstrates each problem (assuming it's feasible)? These bugs haven't actually been assigned to anyone; do you need a mod_isapi maintainer? (Hmm... http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=106854151316221&w=2 may explain a good deal. Sounds like Bill is just plain busy.)
I've looked at the patches previously and ISTR that they looked reasonable to me. But with zero experience in ISAPI-land I thought it would be best if wrowe have a look ;) But it is time to move forward, so I'll go through them again and commit what I'm reasonable sure of to Apache 2.1-dev and suggest for a backport. Thanks for the ping ;)
Sounds good. Please let me know if you find anything that gets in the way of checking them in; I'm happy to do what I can do address any problems. The backport should be trivial, since the trunk and 2.0 branch have been kept in sync to the extent possible. I imagine the real question is whether the maintainers are comfortable with the changes.
patch committed to 2.1-dev, thanks!! will propose shortly that it be merged to the stable branch
+1 on backport here ... if you validated against IIS. Since ISAPI is an abandoned API, pretty much defined by 'whatever it is IIS does', I can go along with this patch. It might confuse someone on a non-Win32 mod_isapi - but since crlf is consistent in HTTP headers, I'll agree with Jesse here. Thanks again :)
You're welcome! I'm glad to see this getting in. I bumped up against this when header parsing code that worked under IIS didn't work under mod_isapi. I generated the patch from the change I made to mod_isapi to send headers that I could parse - that is, headers similar to the ones I got from IIS. I think that's about as much validation as is possible under the circumstances. (Barring risking your legal neck by looking for IIS source in the recently leaked Windows code, that is.) Out of curiosity, do you know if anyone is successfully using mod_isapi on a non-Win32 platform?
Any progress on backporting this patch and the ones for 20617 and 20619? It has been a couple of months since 2.0.49 came out; it would be nice if these could make it into 2.0.50, whenever that happens.
All three patches have two votes for backport, awaiting one more :( For any developers who see this, please consider these merge requests in the STATUS file. No special mod_isapi knowledge necessary. * mod_isapi: GetServerVariable("ALL_RAW") returned the wrong buffer size. PR 20617 [Jesse Pelton <jsp pkc.com>] http://cvs.apache.org/viewcvs.cgi/httpd-2.0/modules/arch/win32/mod_isapi.c?r1=1.96&r2=1.97 +1: trawick, stoddard * mod_isapi: send_response_header() failed to copy status string's last character. PR 20619. [Jesse Pelton <jsp pkc.com>] http://cvs.apache.org/viewcvs.cgi/httpd-2.0/modules/arch/win32/mod_isapi.c?r1=1.97&r2=1.98 +1: trawick, stoddard * mod_isapi: GetServerVariable returned improperly terminated header fields given "ALL_HTTP" or "ALL_RAW". PR 20656. [Jesse Pelton <jsp pkc.com>] http://cvs.apache.org/viewcvs.cgi/httpd-2.0/modules/arch/win32/mod_isapi.c?r1=1.98&r2=1.99 +1: trawick, stoddard