Project

General

Profile

Bug #1339

lighttpd doesn't set empty QUERY_STRING in cgi environment

Added by alexo over 9 years ago. Updated about 1 year ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
Category:
mod_cgi
Target version:
Start date:
Due date:
% Done:

0%

Missing in 1.5.x:

Description

We found using some proprietary billing software (like http://www.billmax.com/) REQUIRES QUERY_STRING to be set in any cases regardless is it empty.

Suggesting patch fixes this lighttpd behaviour.

lighttpd_empty_query_string.patch View - Patch #1 (540 Bytes) alexo, 2007-09-06 05:57

Associated revisions

Revision f1681ca2 (diff)
Added by gstrauss about 1 year ago

[mod_cgi] always set QUERY_STRING (fixes #1339)

(thx alexo)

x-ref:
"lighttpd doesn't set empty QUERY_STRING in cgi environment"
https://redmine.lighttpd.net/issues/1339

History

#1 Updated by stbuehler over 8 years ago

  • Status changed from New to Fixed
  • Resolution set to invalid

CGI spec:

"This variable should always be set when there is query information"

Sorry, fix your app (if you paid for it they should care about it).

#2 Updated by stbuehler over 8 years ago

  • Status changed from Fixed to Invalid

#3 Updated by Spida over 6 years ago

  • Status changed from Invalid to Reopened

According to rfc3875 [1]:
"The server MUST set this variable; if the Script-URI does not include a query component, the QUERY_STRING MUST be defined as an empty string ("")."
so we are NOT rfc-compliant there.

[1] http://www.rfc-editor.org/rfc/rfc3875.txt

#4 Updated by BaconSpot almost 4 years ago

This issue is still pending. A similar problem occurs with the widespread open source mailing list software mailman, when accessing the web interface over cgi. Cf. https://bugs.launchpad.net/mailman/+bug/1160647

#5 Updated by gstrauss about 1 year ago

  • Description updated (diff)
  • Target version changed from 1.5.0 to 1.4.x

This affects 1.4.x, too. Attached 2-line patch is correct. Please apply.

#6 Updated by alexo about 1 year ago

gstrauss wrote:

This affects 1.4.x, too. Attached 2-line patch is correct. Please apply.

I would suggest to find some better web-server software replacement (with adequate maintenance). Unless you want to wait 10 years more lol

#7 Updated by gstrauss about 1 year ago

Sorry you've had to wait so long, alexo. I can assure you this will now get resolved much sooner than that. :)

darix suggested a simpler patch to unconditionally set QUERY_STRING, as specified by the RFCs.

diff --git a/src/mod_cgi.c b/src/mod_cgi.c
index d3e0297..c4a4d2e 100644
--- a/src/mod_cgi.c
+++ b/src/mod_cgi.c
@@ -947,9 +947,7 @@ static int cgi_create_env(server *srv, connection *con, plugin_data *p, buffer *
                        cgi_env_add(&env, CONST_STR_LEN("PATH_INFO"), CONST_BUF_LEN(con->request.pathinfo));
                }
                cgi_env_add(&env, CONST_STR_LEN("REDIRECT_STATUS"), CONST_STR_LEN("200"));
-               if (!buffer_string_is_empty(con->uri.query)) {
-                       cgi_env_add(&env, CONST_STR_LEN("QUERY_STRING"), CONST_BUF_LEN(con->uri.query));
-               }
+               cgi_env_add(&env, CONST_STR_LEN("QUERY_STRING"), CONST_BUF_LEN(con->uri.query));
                if (!buffer_string_is_empty(con->request.orig_uri)) {
                        cgi_env_add(&env, CONST_STR_LEN("REQUEST_URI"), CONST_BUF_LEN(con->request.orig_uri));
                }

#8 Updated by alexo about 1 year ago

gstrauss wrote:

Sorry you've had to wait so long, alexo. I can assure you this will now get resolved much sooner than that. :)

darix suggested a simpler patch to unconditionally set QUERY_STRING, as specified by the RFCs.

[...]

Maybe it is simpler, but can't say it is safer: you are dropping empty string check. NPE or buffer overload is just waiting here.

#9 Updated by stbuehler about 1 year ago

The linked RFC is just "informational", and happens to collide with the "CGI/1.1" name used in http://web.archive.org/web/20100127161358/http://hoohoo.ncsa.illinois.edu/cgi/ - which, afaict, is the CGI "origin". I cannot say who came up with the name "CGI/1.1" first, but I'd say that those who defined the first version should be considered authoritative.

As the original CGI spec page is down now, one can probably argue that it is better to follow the RFC thing now, even if it is stupid and broken (why require an empty environment var? that should be the default value...).

The modified patch doesn't work because cgi_env_add will return early on NULL key / value (not sure what else would change if cgi_env_add would be changed).

#10 Updated by gstrauss about 1 year ago

As stbuehler noted, the quick patch I posted above won't work since cgi_env_add will return early if it receives a NULL value.

The original patch from alexo does work. Please apply.

#11 Updated by gstrauss about 1 year ago

  • Status changed from Reopened to Patch Pending

The original two-line patch from alexo is repeated below.

--- mod_cgi.c.orig    Wed Sep  5 03:48:30 2007
+++ mod_cgi.c    Wed Sep  5 03:29:04 2007
@@ -842,6 +842,8 @@
         cgi_env_add(&env, CONST_STR_LEN("REDIRECT_STATUS"), CONST_STR_LEN("200"));
         if (!buffer_is_empty(con->uri.query)) {
             cgi_env_add(&env, CONST_STR_LEN("QUERY_STRING"), CONST_BUF_LEN(con->uri.query));
+        } else {
+            cgi_env_add(&env, CONST_STR_LEN("QUERY_STRING"), CONST_STR_LEN(""));
         }
         if (!buffer_is_empty(con->request.orig_uri)) {
             cgi_env_add(&env, CONST_STR_LEN("REQUEST_URI"), CONST_BUF_LEN(con->request.orig_uri));

#12 Updated by gstrauss about 1 year ago

  • Target version changed from 1.4.x to 1.4.40

#13 Updated by gstrauss about 1 year ago

  • Status changed from Patch Pending to Fixed

committed in f1681ca2

Also available in: Atom