Hi Sven, I'm unsure about where to set the dynamic variables. I don't really want to do that in my WAApplication as it's not application logic. I could subclass ZnZincStreamingServerAdaptor... Do you have any suggestions?
Cheers, Max > On 11 May 2017, at 13:05, Max Leske <[email protected]> wrote: > >> >> On 11 May 2017, at 11:49, Sven Van Caekenberghe <[email protected]> wrote: >> >> Hi Max, >> >> First thank you for the feedback, the discussion so far and your patches. I >> studied them carefully. >> >> Still, I decided to take another road, implementation wise. >> >> You see, the reason a global setting does not make sense is that there can >> (and are) multiple Zn clients and servers active in the same image, and each >> should be independent, not be influenced by configuration changes in others. >> This is why the use of dynamic variables fits so well. >> >> I refactored the current situation a bit and added the necessary high level >> hooks. I moved the default to each dynamic variable class itself, removed it >> from ZnConstants, and added options to both ZnClient and ZnServer. >> >> Please test (code committed in bleedingEdge) and let me know if it works for >> you (especially then #defaultEncoder part). Maybe we have to iterate more >> over this to fully support your use case. > > I will. Thanks Sven! > >> >> Regards, >> >> Sven >> >> PS: One resource limit not yet moved under the new scheme is the maximum >> line length (currently set to 4096). >> >>> On 6 May 2017, at 15:30, Max Leske <[email protected]> wrote: >>> >>>> >>>> On 5 May 2017, at 17:11, Sven Van Caekenberghe <[email protected]> wrote: >>>> >>>> Max, >>>> >>>>> On 5 May 2017, at 16:59, Max Leske <[email protected]> wrote: >>>>> >>>>> Hi, >>>>> >>>>> I'm performing a legal request that has more than 4000 parameters. This >>>>> causes the Zinc server to return 400: Bad Request because >>>>> ZnMultiValueDictionary is limited to 256 entries by default. >>>>> >>>>> The dictionary has the option to remove the limit or to adjust it with a >>>>> dynamic variable. Unfortunately, I don't see any way to properly >>>>> configure this without monkey patching Zinc. Ideally, I'd like to remove >>>>> the limit (which can't be done through the dynamic variable by the way >>>>> because when the dynamic variable answers nil, the default will be set to >>>>> 256). >>>>> >>>>> The first thing that comes to mind is to move this setting to >>>>> ZnConstants, but then I don't see any way to configure ZnConstants either >>>>> (ZnConstants is referenced directly by its users). Maybe ZnConstants >>>>> could be changed to hold a concrete constants class (itself by default). >>>>> >>>>> In any case, I think this setting should be configurable and the >>>>> configuration should be possible through one single entry point, together >>>>> with options like #codec. >>>>> >>>>> Thoughts? >>>>> >>>>> Cheers, >>>>> Max >>>> >>>> You are the first one to complain about this limit. >>>> >>>> This one and other resource limits exist to protect the client/server >>>> against abuse and attacks. >>> >>> I'm aware of that but have not other way to do this right now. We have >>> mod_security with a higher value configured for that. >>> >>>> >>>> I think what is needed is something like >>>> ZnServer>>#withMaximumEntitySizeDo: which uses the server option >>>> #maximumEntitySize. Would that work for you, you think ? >>> >>> Yes, it would. I've implemented the change, closely following the semantics >>> of #withMaximumEntitySizeDo:. While I was working on that I discovered >>> another problem I had which I also fixed by dispatching to ZnConstants. The >>> problem being, that I set the codec on the server to GRNullCodec, but >>> ZnPercentEncoder would still always use a ZnUTF8Encoder to decode requests. >>> The result was an error when the server tried to write wide strings onto >>> the stream (usually ZnUTF8Encoder: UTF-8 -> WideString, GRPharoUtf8Codec: >>> WideString -> UTF-8). >>> >>> I've attached the patches. >>> >>> A couple of notes: >>> - #defaultMaximumNumberOfDictionaryEntries and #maximumDictionaryEntries: >>> have the same implementation, which is unnecessary in my opinion but I've >>> copied the code from #maximumEntitySize:. >>> - I'm not sure whether storing the option a second time on ZnServer makes >>> sense, but, again, I stuck to the existing implementation >>> - #withMaximumNumberOfDictionaryEntriesDo: also checks the dynamic >>> variable, which is overkill I think, as ZnConstants does the same thing. >>> Again, I stuck to the existing implementation (which means that ZnConstants >>> is actually thread agnostic and references to dynamic variables should >>> probably all be on ZnConstants) >>> >>> >>> Cheers, >>> Max >>> >>>> >>>> Sven
