[Zinc] How do I set the entry limit for ZnMultiValueDictionary?

> On 5 May 2017, at 16:59, Max Leske <[hidden email]> 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

> On 6 May 2017, at 15:30, Max Leske <[hidden email]> wrote:
>
>>
>> On 5 May 2017, at 17:11, Sven Van Caekenberghe <[hidden email]> wrote:
>>
>> Max,
>>
>>> On 5 May 2017, at 16:59, Max Leske <[hidden email]> 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

> On 11 May 2017, at 11:49, Sven Van Caekenberghe <[hidden email]> 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.

>
> 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 <[hidden email]> wrote:
>>
>>>
>>> On 5 May 2017, at 17:11, Sven Van Caekenberghe <[hidden email]> wrote:
>>>
>>> Max,
>>>
>>>> On 5 May 2017, at 16:59, Max Leske <[hidden email]> 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
>
>

> On 11 May 2017, at 14:00, Max Leske <[hidden email]> 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 <[hidden email]> wrote:
>>
>>>
>>> On 11 May 2017, at 11:49, Sven Van Caekenberghe <[hidden email]> 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 <[hidden email]> wrote:
>>>>
>>>>>
>>>>> On 5 May 2017, at 17:11, Sven Van Caekenberghe <[hidden email]> wrote:
>>>>>
>>>>> Max,
>>>>>
>>>>>> On 5 May 2017, at 16:59, Max Leske <[hidden email]> 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
>

> On 11 May 2017, at 14:06, Sven Van Caekenberghe <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> wrote:
>>>
>>>>
>>>> On 11 May 2017, at 11:49, Sven Van Caekenberghe <[hidden email]> 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 <[hidden email]> wrote:
>>>>>
>>>>>>
>>>>>> On 5 May 2017, at 17:11, Sven Van Caekenberghe <[hidden email]> wrote:
>>>>>>
>>>>>> Max,
>>>>>>
>>>>>>> On 5 May 2017, at 16:59, Max Leske <[hidden email]> 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
>>
>
>

> On 11 May 2017, at 14:24, Max Leske <[hidden email]> wrote:
>
> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> wrote:
>>>>
>>>>>
>>>>> On 11 May 2017, at 11:49, Sven Van Caekenberghe <[hidden email]> 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 <[hidden email]> wrote:
>>>>>>
>>>>>>>
>>>>>>> On 5 May 2017, at 17:11, Sven Van Caekenberghe <[hidden email]> wrote:
>>>>>>>
>>>>>>> Max,
>>>>>>>
>>>>>>>> On 5 May 2017, at 16:59, Max Leske <[hidden email]> 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
>>>
>>
>>
>
>

>
>> On 11 May 2017, at 14:24, Max Leske <[hidden email]> wrote:
>>
>> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> wrote:
>>>>>
>>>>>>
>>>>>> On 11 May 2017, at 11:49, Sven Van Caekenberghe <[hidden email]> 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 <[hidden email]> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On 5 May 2017, at 17:11, Sven Van Caekenberghe <[hidden email]> wrote:
>>>>>>>>
>>>>>>>> Max,
>>>>>>>>
>>>>>>>>> On 5 May 2017, at 16:59, Max Leske <[hidden email]> 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
>>>>
>>>
>>>
>>
>>
>
>