> 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 > >
