Allow configuring or disabling max cookie size check#1109
Allow configuring or disabling max cookie size check#1109davidism merged 3 commits intopallets:masterfrom davidism:max-cookie-size
Conversation
add max_cookie_size attr to BaseResponse add changelog for #780
| supported by all browsers. | ||
| """ | ||
| self.headers.add('Set-Cookie', dump_cookie(key, | ||
| value=value, |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
LGTM, might need a testcase for custom values though |
|
The error is slightly confusing because you can pass 4093 bytes as the value and still get the error, since the options are added to the payload. I don't think we need to change it, just pointing it out. |
|
I think we should elaborate the error message because of this.
In Flask it might make sense to only enable this limit in developer mode, and
rather try not to crash in production.
…On Tue, Apr 18, 2017 at 08:37:40AM -0700, David Lord wrote:
The error is slightly confusing because you can pass 4093 bytes as the value and still get the error, since the options are added to the payload. I don't think we need to change it, just pointing it out.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#1109 (comment)
|
|
Maybe this should be a warning? Could use a filter to turn it into an error in Flask debug mode. |
|
How about detecting the before and after version, then warning
|
|
Sounds good
…On April 18, 2017 6:24:54 PM GMT+02:00, David Lord ***@***.***> wrote:
How about detecting the before and after version, then warning "Cookie
too large: value was {value_size} bytes but header required
{header_size - value_size} extra bytes. The final size was
{header_size} but the limit is {max} bytes. Browsers may silently
ignore cookies larger than this."
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#1109 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
Thanks @davidism for making this configurable, there should be need to override the limit of 4k per cookie in most cases, but yes, some people might want to change it. |
Continue #780
dump_cookiemax_sizemax_cookie_sizeattr toBaseResponse