[Django] #33245: utils.urlize isn't thread-safe
Django
-----------------------------------------+------------------------
Reporter: Tim McCurrach | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
Since [changeset:"e567670b1abe61af4acfaa6a6a7e92a7acfa8b00" e567670b]
`utils.urlize` isn't thread safe!
=== To replicate bug: ===
1. Write 2 views that both use the `urlizetrunc` tag a large number of
times (between 10^4^ and 10^5^ was enough on my computer).
2. Use different url length limits (for truncation) for each view
3. Load the 2 views simultaneously
The resulting pages will have inconsistent url limits, as the
`trim_url_limit` value from one view leaks over to the other.
=== The cause ===
Since `urlize` was changed to become class-based `trim_url_limit`,
`nofollow`, `autoescape`, and `trim_url_limit` are stored as instance
attributes. `Urlizer` is instantiated just once and then used within
`urlize` which allows for these values to be shared between function
calls.
=== The solution ===
- The obvious solution would be to pass the values listed above directly
to `handle_word` so that they are not stored on the instance.
- My only question is: Does removing these values from the class instance
nullify the ease of customisation the original ticket brought about? If
this is the case, the better solution might just be to revert the change.
- An alternative approach would be to create a new instance of `Urlizer`
on each call of `urlize`, but since this can be called many times in a
single request, this would likely have a performance impact.
--
Ticket URL: <https://code.djangoproject.com/ticket/33245>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Tim McCurrach | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: dev
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Tim McCurrach:
Old description:
New description:
Since [changeset:"e567670b1abe61af4acfaa6a6a7e92a7acfa8b00" e567670b]
`utils.html.urlize` isn't thread safe!
=== To replicate bug: ===
1. Write 2 views that both use the `urlizetrunc` tag a large number of
times (between 10^4^ and 10^5^ was enough on my computer).
2. Use different url length limits (for truncation) for each view
3. Load the 2 views simultaneously
The resulting pages will have inconsistent url limits, as the
`trim_url_limit` value from one view leaks over to the other.
=== The cause ===
Since `urlize` was changed to become class-based `trim_url_limit`,
`nofollow`, `autoescape`, and `trim_url_limit` are stored as instance
attributes. `Urlizer` is instantiated just once and then used within
`urlize` which allows for these values to be shared between function
calls.
=== The solution ===
- The obvious solution would be to pass the values listed above directly
to `handle_word` so that they are not stored on the instance.
- My only question is: Does removing these values from the class instance
nullify the ease of customisation the original ticket brought about? If
this is the case, the better solution might just be to revert the change.
- An alternative approach would be to create a new instance of `Urlizer`
on each call of `urlize`, but since this can be called many times in a
single request, this would likely have a performance impact.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:1>
Django
-------------------------------+--------------------------------------
Reporter: Tim McCurrach | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: dev
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:2>
Django
---------------------------------+------------------------------------
Reporter: Tim McCurrach | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Claude Paroz (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the report!
> The obvious solution would be to pass the values listed above directly
to `handle_word` so that they are not stored on the instance.
Agreed, it's should be enough to pass values to the underlying methods.
> My only question is: Does removing these values from the class instance
nullify the ease of customisation the original ticket brought about?
I don't think so. As far as I'm aware
[https://github.com/django/django/blob/f38458fe56bf8850da72a924bd2e8ff59c6adf06/django/utils/html.py#L230-L241
all important parameters] will still be customizable.
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:3>
Django
Reporter: Tim McCurrach | Owner: Tim McCurrach
Type: Bug | Status: assigned
Component: Utilities | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* owner: nobody => Tim McCurrach
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15040 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:4>
Django
---------------------------------+-----------------------------------------
Reporter: Tim McCurrach | Owner: Tim McCurrach
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
[HS] Mariusz, do you know why I'm not receiving any mail when you CC me?
It's not a problem since I generally follow the timeline, but I still
wonder.
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:5>
Django
---------------------------------+-----------------------------------------
Reporter: Tim McCurrach | Owner: Tim McCurrach
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim McCurrach):
Replying to [comment:5 Claude Paroz]:
> [HS] Mariusz, do you know why I'm not receiving any mail when you CC me?
It's not a problem since I generally follow the timeline, but I still
wonder.
FWIW, I'm not receiving notifications about this ticket either. Strange...
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:6>
Django
Reporter: Tim McCurrach | Owner: Tim
| McCurrach
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Tim McCurrach | Owner: Tim
| McCurrach
Component: Utilities | Version: dev
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ad81b606a2b5276397460a654fc7ad901a54b91e" ad81b606]:
{{{
#!CommitTicketReference repository=""
revision="ad81b606a2b5276397460a654fc7ad901a54b91e"
Fixed #33245 -- Made django.utils.html.urlize() thread-safe.
Regression in e567670b1abe61af4acfaa6a6a7e92a7acfa8b00.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Tim McCurrach | Owner: Tim
| McCurrach
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"1f9874d4ca3e7376036646aedf6ac3060f22fd69" 1f9874d4]:
{{{
#!CommitTicketReference repository=""
revision="1f9874d4ca3e7376036646aedf6ac3060f22fd69"
Refs #33245 -- Minor edits to django.utils.html.urlize() changes.
Follow up to ad81b606a2b5276397460a654fc7ad901a54b91e.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33245#comment:9>