[Django] #32601: Optimize split_domain_port() by leveraging the regex

3 views
Skip to first unread message

Django

unread,
Mar 29, 2021, 5:13:28 AM3/29/21
to django-...@googlegroups.com
#32601: Optimize split_domain_port() by leveraging the regex
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: nobody
Jerdonek |
Type: | Status: new
Cleanup/optimization |
Component: HTTP | Version: dev
handling | Keywords:
Severity: Normal | split_domain_port,regex
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I noticed that almost all of the pure Python can be removed from Django's
`split_domain_port()` function:
https://github.com/django/django/blob/2f13c476abe4ba787b6cb71131818341911f43cc/django/http/request.py#L643-L650
by modifying the `host_validation_re` regex slightly and then accessing it
via its groups:
https://github.com/django/django/blob/2f13c476abe4ba787b6cb71131818341911f43cc/django/http/request.py#L26
Currently, the function uses the regex to determine if there's a match,
and then uses pure Python to reparse out the components (work which the
regex has already done for the most part).

I can do this once accepted.

--
Ticket URL: <https://code.djangoproject.com/ticket/32601>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Mar 29, 2021, 5:48:20 AM3/29/21
to django-...@googlegroups.com
#32601: Optimize split_domain_port() by leveraging the regex
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
split_domain_port,regex | Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => duplicate


Comment:

Thanks for the suggestion, this optimization has already been proposed as
a part of #31354, see [https://github.com/django/django/pull/12844 PR]. I
think we can keep it there and merge it before the main fix.

--
Ticket URL: <https://code.djangoproject.com/ticket/32601#comment:1>

Django

unread,
Mar 29, 2021, 6:20:29 AM3/29/21
to django-...@googlegroups.com
#32601: Optimize split_domain_port() by leveraging the regex
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
split_domain_port,regex | Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Chris Jerdonek):

Thanks for the pointer. That PR doesn't contain all the optimizations I
had in mind, though. There is more pure Python that can be removed.

--
Ticket URL: <https://code.djangoproject.com/ticket/32601#comment:2>

Django

unread,
Mar 29, 2021, 6:23:17 AM3/29/21
to django-...@googlegroups.com
#32601: Optimize split_domain_port() by leveraging the regex
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
split_domain_port,regex | Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Chris Jerdonek):

I was also going to add more tests as part of my change because currently
there is only one test case (testing dot removal):
https://github.com/django/django/blob/2f13c476abe4ba787b6cb71131818341911f43cc/tests/requests/tests.py#L813-L816

--
Ticket URL: <https://code.djangoproject.com/ticket/32601#comment:3>

Reply all
Reply to author
Forward
0 new messages