That's perfect, with one exception: it is still not possible to set the number of maximum dictionary entries to unlimited. In ZnMultiValueDictionary you have a nil check for the unlimited case but the dynamic variable will always default to the default value because DynamicVariable uses the default when the variable has a nil value. Not sure what the best solution is there...
Max > On 11 May 2017, at 14:06, Sven Van Caekenberghe <[email protected]> wrote: > > ZnZincServerAdaptor and subclasses expose a #server accessor that gives you > access to the configured Zn server. You should be able to grab that and set > additional options (#maximumEntitySize: #maximumNumberOfDictionaryEntries: > #defaultEncoder:) in your setup code. > >> On 11 May 2017, at 14:00, Max Leske <[email protected]> wrote: >> >> 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 >> > >
