[Seaside] Bug in WAAbstractFileLibrary

[Joachim Tuchel]

Hi Johan,

you are right, I tried

'' first

in Pharo 2.0 and got a SubscriptOutOfBounds.
I remember I had some other String operation that reacted differently in 
Pharo than in VAST. This was also related to some encoding/decoding 
problem. So I almost would've not told you about the problem because I 
was convinced you wouldn't see the problem on Pharo...

So I am glad I did.

You are, of course, perfectly right. I shouldn't use WAFileLibrary to 
serve things like JQuery. But you know how things are when you've got 
lots of other problems. Outsourcing that job to Apache is on my list, 
somewhere... down there ... ;-) I am not seeing any performance 
bottlenecks and the hassle of deploying my files to some directory and 
write a rewrite rule and stuff seems unjustified. And I like the thought 
that there is this last resort should I see performance problems ;-)

Joachim





Am 14.04.14 09:52, schrieb Johan Brichau:
> Hi Joachim,
>
> Thank you for the report and the suggested fix.
> I think that returning a 404 rather than throwing an exception is indeed a good suggestion.
> In Pharo 3, at least, this also throws an exception (it is not ignored), so the behaviour is the same.
>
> But most importantly, I think you should not use a WAFileLibrary in production.
> Not really because of these problems, but because serving static files can be done way more efficiently by your front-end web server.
>
> best regards,
> Johan
>
> On 14 Apr 2014, at 06:01, Joachim Tuchel <jtuchel at objektfabrik.de> wrote:
>
>> Hi there,
>>
>> over the last few nights, our Seaside Application was bombarded with requests that were formed like this:
>>
>> /files/JQUiDeploymentLibrary/%29.find%28
>>
>> The attacks did also try other javascript expressions.
>>
>> Unfortunately, WAAbstractFileLibrary reacts to this by throwing a primitive failed on VA Smalltalk in WAAbstractFileLibrary class>>#asSelector:, because the javascript expression cannot be interpreted as a filename.
>>
>> Here's an excerpt of our walkback that shows what's going on.
>>
>> String(Object)>>#primitiveFailed
>>   receiver = ''
>> String>>#at:
>>   receiver = ''
>>   arg1 = 1
>> String(SequenceableCollection)>>#first
>>   receiver = ''
>> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#asSelector:
>>   receiver = JQUiDeploymentLibrary
>>   arg1 = ').find('
>>   temp1 = ''
>>   temp2 = nil
>> JQUiDeploymentLibrary(WAAbstractFileLibrary)>>#asSelector:
>>   receiver = a JQUiDeploymentLibrary
>>   arg1 = ').find('
>> JQUiDeploymentLibrary(WAFileLibrary)>>#handle:
>>   receiver = a JQUiDeploymentLibrary
>>   arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>>   temp1 = ').find('
>>   temp2 = nil
>>   temp3 = nil
>> JQUiDeploymentLibrary class(WAAbstractFileLibrary class)>>#handle:
>>   receiver = JQUiDeploymentLibrary
>>   arg1 = a WARequestContext url: '/files/JQUiDeploymentLibrary/%29.find%28'
>>
>> I am on the road and have no pharo/seaside image with me, but if I remember correctly, pharo does not throw an exception when you ask an empty string for its #first character, I seem to remember it just returns nil. VA Smalltalk does throw an exception. It does not stop working, so this is not a critical problem.
>>
>> However, I think an additional check in #asSelector: wouldn't hurt because then the result is an http error code 404, which can either be returned to the client or removed by filters like mod_security.
>>
>> So here is a fix for WAAbstractFileLibrary class>>asSelector: that I suggest for inclusion in Seaside, even if it is unnecessary for Pharo:
>>
>> asSelector: aFilename
>>     | mainPart extension |
>>     mainPart := (aFilename copyUpToLast: $.)
>>         select: [ :each | each isAlphaNumeric ].
>>
>>     mainPart isEmptyOrNil ifTrue: [^nil].
>>
>>     [ mainPart first isDigit ]
>>         whileTrue: [ mainPart := mainPart allButFirst ].
>>     extension := (aFilename copyAfterLast: $.) asLowercase capitalized.
>>     ^ (mainPart, extension) asSymbol
>>
>> Joachim
>>
>>
>>
>> _______________________________________________
>> seaside mailing list
>> seaside at lists.squeakfoundation.org
>> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
> _______________________________________________
> seaside mailing list
> seaside at lists.squeakfoundation.org
> http://lists.squeakfoundation.org/cgi-bin/mailman/listinfo/seaside
>