HTTP POST handling non-compliant, and failing

[rob@…]

HTTP POST handling issues:

Registered as a single bug because a single fix is required.

HTTP POST handling (cgiutil.c) has a number of problems.

1) requires header Content-Length to be set - this is unfriendly and also not 
compliant unless you somehow force the server also to advertise it is 
restricted to HTTP/1.0

2) use of function fread causes pain - cannot have perfectly legal newlines in 
XML encoding:

eg

<?xml version="1.0" encoding="UTF-8"?>
<wfs:GetFeature maxFeatures="10" .....

fails, whereas

<?xml version="1.0" encoding="UTF-8"?><wfs:GetFeature maxFeatures="10" ....

works

3) use of Content-Length header to malloc memory without checks on sanity 
(size) or success of malloc not so good (have to make sure that fix does not 
introduce buffer overrun vulnerabilities)

4) does not wait for content to be supplied - if you fail to flush() the URL 
output connection immediately after writing it dies with a non-compliant effor 
message - having fallen through processing out of OGC handlers
(not sure why fread does not block to be honest)


5) on fall through, or other errors described, does not respond with valid 
error format

[dmorissette]

I understand the issue with #1 and #3 and they need to be fixed.

#2: What's the link between fread and newlines?  This sounds more like a XML
parsing issue.

#4: I suspect the problem would be with the client (or the HTTP server)
producing an EOF prematurely. I have never noticed this kind of issue before. 
Can you please at least provide a way to reproduce this?

#5: Since the request hasn't been decoded yet when those errors happen, there is
no way for MapServer to know in which format to spit out errors.  We could
possibly add a flag in the mapfile to override error format, but then regular
CGI apps using the same mapfile would produce XML errors.

Adding Frank and Steve to the CC.

[fwarmerdam]

It is irritating me while debugging unrelated problems, so I am going to 
fix the requirement for CONTENT_LENGTH. 

[rob@…]

Thanks Frank

can you also please make sure in your testing that issue #2 is resolved - i.e. 
the input is not sensitive to whitespace in the XML!

This is the actual showstopper for interoperability with the current build.

[fwarmerdam]

OK, CONTENT_LENGTH is no longer required. 

There is also a check for malloc() failure on very large CONTENT_LENGTH, but
it doesn't try to avoid accepting large inputs.  Some sorts of operations
could in theory require rather large posts so I hesitant to fiddle with this
aspect. 

I do know know why issue #2 (newlines in XML) is causing you problems.  Can
you provide a testcase that demonstrates this problem?  In theory a simple
mapfile with on inline feature and an appropriate WFS request to read it should
be sufficient to demonstrate, but I am loath to start experimenting with it from
my end.  I hate trying to reproduce produce a test case to reproduce an error
I suspect doesn't really exist. :-)

As Daniel notes, it is hard for us to return errors according to requested
format in url/post processing.  I am not going to try and address that myself.

I'm afraid I don't grasp issue #4.  Is the problem that we try to read the
posted data in a single fread() instead of repeating fread() requests still we
get the feof(stdin)?  In fact, the case of missing content-length does read
till feof() and I could easily make the case with content length do the same
thing if that is the problem.   But I would prefer to verify this is the 
problem you are reporting.  

Finally, while I have tested url encoded post requests with and without
content-length I have not tested XML posted requests at all yet.  If anyone
else can easily test with the code now in CVS that would be appreciated. 

[dmorissette]

Assefa should be able to test the XML-encoded post with and without
content-length with his WFS testsuite.

[sdlime]

Just for my information, are these changes impacting the normal CGI behavior at 
all?

[fwarmerdam]

Steve, 

The changes do affect normal mapserv cgi behaviour *if* POST is being
used.  In fact, that is the case I was testing with. 

[sdlime]

How then? Post's are quite common and are never XML-based with the exception of 
the OGC piece.

[fwarmerdam]

They are affected by working without Content-Length should that occur. 
They also will no longer emit the junk debugging message into the 
returned headers that were defeating content-type identification on some
browsers (such as recent Mozilla's). 

However, for the most part things should continue to work normally.

[dmorissette]

Frank, can we mark this fixed since the main issues are resolved, and it seems
that the pending issues (#2 and #4) require a testcase to reproduce and Rob
could file new bugs if/when he has those test cases?

[dmorissette]

About issue #2: bug 601 reported a potential issue with linefeeds and the XML
parser... perhaps it's related?

[fwarmerdam]

Daniel, 

I am fine with closing this bug report since we can't seem to get a test
case to reproduce the newline and "partial read" problems. 

[dmorissette]

Marking fixed.  Please create new bugs if you can provide test cases for items
#2 and/or #4.

[rob@…]

Sorry about not supplying test cases - have been offline for a bit - Looks like 
bug 601 has embedded another bug with the same issue as #2 to me. Will reopen 
as a brand new bug in its own right with test cases if not fixed. Thanks for 
the effort guys.