WAUrl class >> absolute mismatch with ZnZincServerAdaptor >> #requestUrlFor:

[marianopeck]

Hi guys,

It's quite common to get requests with invalid URLs. For example: http://localhost:8088/config?type=php://filter/resource=http://whsec.us/rfi.php?.

Right now, on Pharo, if you do WAUrl absolute: '/config?type=php://filter/resource=http://whsec.us/rfi.php?' you get a correct WAInvalidUrlSyntaxError. However... ZnZincServerAdaptor >> #requestUrlFor: does not rely on WAUrl class >> absolute (as VA adaptor does) and hence that request does not fail at all. If you go to the browser, and paste that url, it will be simply processed.

I think this goes in conflict with absolute throwing an error for that same URL.

At Instantiations we got already 2 customers which had to hook on VA Seaside adaptor to put an error handler on WAInvalidUrlSyntaxError process: and if that's the case, then answer a bad request. While I will be patching the code in VA port itself, I think we can provide something general at Seaside upstream level. Something like:

process: aNativeRequest
	"Process aNativeRequest by converting to a request context, handling the request, and converting the request context back to a native response. Make sure to release the context after processing, to help the garbage collector and avoid having large streams and sockets hanging around if people still refer to the obsolete request context."

	| context |
	context := nil.
	[ context := self contextFor: aNativeRequest ]
		on: Error
		do: [ :e | 
			^ self errorResponseFromNativeRequest: aNativeRequest error: e ].
	^ [ self
		handle: context;
		responseFrom: context ]
		ensure: [ context destroy ]

And:

errorResponseFromNativeRequest: aNativeRequest error: anError
	"Answer a bad request when there is an error in getting a request context for a given @aNativeRequest.
	By default, we do not include the description of the exception. You may want to subclass this adapter
	and implement your own."

	^ WABufferedResponse new
		badRequest;
		"nextPutAll: anError description;"
		yourself

Finally, I would change ZnZincServerAdaptor >> #requestUrlFor: so that it "validates" the original URL via "WAUrl >> absolute" just to see if it valid or not. If it is not valid, it would just signal a WAInvalidUrlSyntaxError and we will handle it as bad request (with code change above).

What do you think in general? And if you agree, any idea how to implement the fix for Zinc?

Best,

[marianopeck]

After a more careful review, I think errorResponseFromNativeRequest: aNativeRequest error: anError can't really by cross platform and we would need that to be "self subclassResponsability"...

For VA it could be something like:

WASstRequestConverter>> errorResponseFromNativeRequest: aNativeRequest
            "Build an SstHttpResponseHeader object.
            Answer an array with the header and the response contents stream."

            | sstResponseHeader |

            sstResponseHeader := SstHttpResponseHeader new.
            sstResponseHeader setStatus: HttpBadRequest.
            aNativeRequest header responseHeader: sstResponseHeader. "Will be set later in WASstServerAdaptor>>process: again, but here for logging in #logRequestUsingAdaptor:."

            ^Array with: sstResponseHeader with: '' readStream

I guess we can do something equivalent for Zinc adaptor... but this would depend on the adaptor as it depends on what is answered in responseFrom:

[marschall]

The issue was that we parse the result of the Zinc parsing again instead of using the parsed result.

[marschall]

Hello @marianopeck I'm assigning this issue to you, here's why:

You have two ways of how you can implement URL parsing:

1. Have the server parse the URL and translate it to a WAUrl. This is what we tried to do in Zinc but due to a bug ended up parsing it again. This should be fixed with Do not parse result of Zinc URL parsing #1151. If your server already parses a string into an URL object, this is the recommended way.
2. If your server only gives a string for the URL pass the raw untranslated string to WAUrl. On WAInvalidUrlSyntaxError return HTTP 400.

If you believe you have a valid URL which Seaside fails to parse please open a dedicated issue for this.