GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-67041: Allow to distinguish between empty and not defined URI components by serhiy-storchaka · Pull Request #123305 · python/cpython

Conversation

serhiy-storchaka commented Aug 25, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Changes in the urllib.parse module:

Add option ~~allow_none~~ missing_as_none in urlparse(), urlsplit() and urldefrag(). If it is true, represent not defined components as None instead of an empty string.
Add option keep_empty in urlunparse() and urlunsplit(). If it is true, keep empty non-None components in the resulting string. By default it is the same as the allow_none value for the result of the urlparse() and urlsplit() calls.
~~Add option keep_empty in the geturl() method of DefragResult, SplitResult, ParseResult and the corresponding bytes counterparts.~~

Issue: urllib.parse wrongly strips empty #fragment, ?query, //netloc #67041

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot mentioned this pull request

Aug 25, 2024

urllib.parse wrongly strips empty #fragment, ?query, //netloc #67041

Closed


          pythongh-67041: Allow to distinguish between empty and not defined UR…

a60c9be

…I components

Changes in the urllib.parse module:

* Add option allow_none in urlparse(), urlsplit() and urldefrag(). If
  it is true, represent not defined components as None instead of an
  empty string.
* Add option keep_empty in urlunparse() and urlunsplit(). If it is
  true, keep empty non-None components in the resulting string.
* Add option keep_empty in the geturl() method of DefragResult,
  SplitResult, ParseResult and the corresponding bytes counterparts.

serhiy-storchaka force-pushed the urllib-parse-allow-none branch from 7032015 to a60c9be Compare

August 31, 2024 09:55

serhiy-storchaka added 3 commits

September 2, 2024 15:14


          Document the things.

a1dbfa6


          Merge branch 'main' into urllib-parse-allow-none

b50b778


          Preserve the status of allow_none in results.

eaa9ce6

serhiy-storchaka marked this pull request as ready for review

November 27, 2024 11:16

bedevere-app Bot added the awaiting core review label

Nov 27, 2024

serhiy-storchaka requested a review from orsenthil

November 27, 2024 11:19

serhiy-storchaka commented Nov 27, 2024

Copy link

Copy Markdown

Member Author

It is now ready to review. The status of allow_none is now saved in the DefragResult, SplitResult and ParseResult objects, so in most cases there is no need to pass the keep_empty argument. geturl() no longer needs the keep_empty parameter.

Unfortunately, these objects now have __dict__ and no longer immutable. This is because non-empty __slots__ is not compatible with tuple subclasses. This is a separate complex issue. I'll try to find a solution of it, but it may be difficult.

The long term plan is to make allow_none True by default, and later deprecate allow_none=False. keep_empty=False can still be useful.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka marked this pull request as draft

November 28, 2024 09:14

bedevere-app Bot removed the awaiting core review label

Nov 28, 2024


          Preserve _keep_empty in copying and encoding.

e5c31dd

serhiy-storchaka marked this pull request as ready for review

December 5, 2024 11:10

bedevere-app Bot added the awaiting core review label

Dec 5, 2024

serhiy-storchaka commented Dec 5, 2024

Copy link

Copy Markdown

Member Author

I am sorry, I forget to copy the _keep_empty attribute in copying/encoding/decoding methods. Now the PR is ready for review.

@orsenthil, @barneygale, could you please make a review?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

barneygale self-requested a review

December 5, 2024 14:03


          Merge branch 'main' into urllib-parse-allow-none

78bdc13

serhiy-storchaka requested a review from AA-Turner as a code owner

November 14, 2025 18:58


          Remove redundant import.

5846bf2

merwok commented Nov 17, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

I think that allow_none is not the correct name: this is not about allowing nones in the input (like how allow_fragments allows fragment in the input), but about controlling the data returned.

Possible alternatives:

use_none (short)
missing_as_none
use_none_for_empty

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka commented Nov 17, 2025

Copy link

Copy Markdown

Member Author

Maybe missing_as_empty? In future the behavior will be changed to use None for missing by default.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

merwok commented Nov 17, 2025

Copy link

Copy Markdown

Member

I think it should be missing_as_empty_string (short for return missing values as empty strings), which is long!

To change the default in the future, do you plan on adding a warning first?
I could see many projects not paying attention to the new param and getting surprised (or silent bugs) a few years after.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka commented Nov 17, 2025

Copy link

Copy Markdown

Member Author

To change the default in the future, do you plan on adding a warning first?

On one hand, a warning will inform everyone about the change (it should be a FutureWarning). You will have to pass explicit True or False to silence it and get your behavior. This how we normally do. On other hand, the warning will unnecessary disturb those who are fine with any behavior. We will discuss this when the time come.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka commented Nov 17, 2025

Copy link

Copy Markdown

Member Author

I think it should be missing_as_empty_string (short for return missing values as empty strings), which is long!

Currently, missing and empty component are not distinguishable. This parameter will allow to distinguish them. This PR adds also the keeps_empty parameter. So, "empty" refers not only to string, but to component. missing_component_as_empty_component, or simply missing_as_empty. Will it work?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka added 2 commits

November 17, 2025 18:22


          Merge branch 'main' into urllib-parse-allow-none

b578c9d


          Rename allow_none to missing_as_none.

7d59b7e

merwok commented Nov 17, 2025

Copy link

Copy Markdown

Member

Ah your’re right, there is empty component and empty string.

missing_as_none is good!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

merwok reviewed

Nov 17, 2025

View reviewed changes

Comment thread

Doc/library/urllib.parse.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

orsenthil previously requested changes

Nov 19, 2025

View reviewed changes

Comment thread

Doc/library/urllib.parse.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka requested review from Fidget-Spinner, ambv, ericsnowcurrently, ezio-melotti, hugovk, iritkatriel, lysnikolaou, markshannon, pablogsal, savannahostrowski, tomasr8 and webknjaz as code owners

January 21, 2026 07:32

serhiy-storchaka added 3 commits

January 21, 2026 09:49


          Merge branch 'main' into urllib-parse-allow-none

e092f1f


          Add compatibility tests for unpickling.

2d98e4c


          Merge branch 'main' into urllib-parse-allow-none

3feb1b1

hugovk removed their request for review

January 21, 2026 21:18


          Merge branch 'main' into urllib-parse-allow-none

5d3be4b

orsenthil commented Jan 22, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Looks like we maintain a patch for the iOS build which is failing to apply with this PR.

  export PATH=/Users/runner/work/cpython/cpython/Apple/iOS/Resources/bin:/Users/runner/work/cpython/cpython/cross-build/arm64-apple-ios-simulator/prefix:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin
  > make -j 3
  arm64-apple-ios-simulator-clang -c -fno-strict-overflow -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -O3 -Wall -mios-version-min=13.0   -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I../../Include/internal -I../../Include/internal/mimalloc -IObjects -IInclude -IPython -I. -I../../Include    -DPy_BUILD_CORE -o Programs/python.o ../../Programs/python.c
  arm64-apple-ios-simulator-clang -c -fno-strict-overflow -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -O3 -Wall -mios-version-min=13.0   -std=c11 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden  -I../../Include/internal -I../../Include/internal/mimalloc -IObjects -IInclude -IPython -I. -I../../Include    -DPy_BUILD_CORE -o Parser/token.o ../../Parser/token.c
  1 out of 1 hunks failed while patching 'Lib/test/test_urlparse.py'

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

orsenthil commented Jan 22, 2026

Copy link

Copy Markdown

Member

This file https://github.com/python/cpython/blob/main/Mac/Resources/app-store-compliance.patch (a meta-patch) needs to be updated for the new behavior (returning None instead of '') for it apply cleanly.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

orsenthil commented Jan 22, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

This file https://github.com/python/cpython/blob/main/Mac/Resources/app-store-compliance.patch should be updated with this content.

diff --git a/Mac/Resources/app-store-compliance.patch b/Mac/Resources/app-store-compliance.patch
index f4b7decc01c..e5d1e7da866 100644
--- a/Mac/Resources/app-store-compliance.patch
+++ b/Mac/Resources/app-store-compliance.patch
@@ -1,21 +1,21 @@
 diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py
-index d6c83a75c1c..19ed4e01091 100644
+index 49a292df934..e1669a0c9b2 100644
 --- a/Lib/test/test_urlparse.py
 +++ b/Lib/test/test_urlparse.py
-@@ -237,11 +237,6 @@ def test_roundtrips(self):
-               '','',''),
+@@ -282,11 +282,6 @@ def test_qs(self, orig, expect):
+               None,None,None),
               ('git+ssh', '[email protected]','/user/project.git',
-               '', '')),
+               None, None)),
 -            ('itms-services://?action=download-manifest&url=https://example.com/app',
--             ('itms-services', '', '', '',
--              'action=download-manifest&url=https://example.com/app', ''),
+-             ('itms-services', '', '', None,
+-              'action=download-manifest&url=https://example.com/app', None),
 -             ('itms-services', '', '',
--              'action=download-manifest&url=https://example.com/app', '')),
+-              'action=download-manifest&url=https://example.com/app', None)),
              ('+scheme:path/to/file',
-              ('', '', '+scheme:path/to/file', '', '', ''),
-              ('', '', '+scheme:path/to/file', '', '')),
+              (None, None, '+scheme:path/to/file', None, None, None),
+              (None, None, '+scheme:path/to/file', None, None)),
 diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py
-index 8f724f907d4..148caf742c9 100644
+index e917f8b61bb..8575172573f 100644
 --- a/Lib/urllib/parse.py
 +++ b/Lib/urllib/parse.py
 @@ -59,7 +59,7 @@

And patch application is successful.

$patch -p1 < Mac/Resources/app-store-compliance.patch
patching file 'Lib/test/test_urlparse.py'
patching file 'Lib/urllib/parse.py'

And

 ./python.exe Lib/test/test_urlparse.py -v

was successful with this change.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka added 4 commits

January 22, 2026 12:53


          Merge branch 'main' into urllib-parse-allow-none

cb60e92


          Fix app-store-compliance.patch.

6bbaece


          Merge remote-tracking branch 'refs/remotes/origin/urllib-parse-allow-…

…none' into urllib-parse-allow-none


          Merge branch 'urllib-parse-allow-none' of github.com:serhiy-storchaka…

b8f2c38

…/cpython into urllib-parse-allow-none

serhiy-storchaka requested a review from a team as a code owner

January 22, 2026 11:04

serhiy-storchaka commented Jan 22, 2026

Copy link

Copy Markdown

Member Author

Thank you @orsenthil. I was confused by this failure. The original issue is interesting: #120522.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka merged commit c5cfcdf into python:main

Jan 22, 2026

47 checks passed

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the awaiting merge label

Jan 22, 2026

serhiy-storchaka deleted the urllib-parse-allow-none branch

January 22, 2026 12:29

thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request

Feb 15, 2026


          pythongh-67041: Allow to distinguish between empty and not defined UR…

64d5157

…I components (pythonGH-123305)

Changes in the urllib.parse module:

* Add option missing_as_none in urlparse(), urlsplit() and urldefrag(). If
  it is true, represent not defined components as None instead of an
  empty string.
* Add option keep_empty in urlunparse() and urlunsplit(). If it is
  true, keep empty non-None components in the resulting string.

Reviewers

gpshead

gpshead left review comments

orsenthil

orsenthil approved these changes

merwok

merwok left review comments

barneygale

Awaiting requested review from barneygale

AA-Turner

Awaiting requested review from AA-Turner

AA-Turner is a code owner

sethmlarson

Awaiting requested review from sethmlarson

pablogsal

Awaiting requested review from pablogsal

lysnikolaou

Awaiting requested review from lysnikolaou

ambv

Awaiting requested review from ambv

ericsnowcurrently

Awaiting requested review from ericsnowcurrently

markshannon

Awaiting requested review from markshannon

tomasr8

Awaiting requested review from tomasr8

Fidget-Spinner

Awaiting requested review from Fidget-Spinner

savannahostrowski

Awaiting requested review from savannahostrowski

iritkatriel

Awaiting requested review from iritkatriel

ezio-melotti

Awaiting requested review from ezio-melotti

webknjaz

Awaiting requested review from webknjaz

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

serhiy-storchaka commented Aug 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka commented Nov 27, 2024

Uh oh!

serhiy-storchaka commented Dec 5, 2024

Uh oh!

merwok commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka commented Nov 17, 2025

Uh oh!

merwok commented Nov 17, 2025

Uh oh!

serhiy-storchaka commented Nov 17, 2025

Uh oh!

serhiy-storchaka commented Nov 17, 2025

Uh oh!

merwok commented Nov 17, 2025

Uh oh!

Uh oh!

Uh oh!

orsenthil commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orsenthil commented Jan 22, 2026

Uh oh!

orsenthil commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka commented Jan 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

serhiy-storchaka commented Aug 25, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

merwok commented Nov 17, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

orsenthil commented Jan 22, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

orsenthil commented Jan 22, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.