Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AnyHttpUrl type appends a slash to the URL #6943

Closed
1 task done
jooola opened this issue Jul 29, 2023 · 7 comments
Closed
1 task done

AnyHttpUrl type appends a slash to the URL #6943

jooola opened this issue Jul 29, 2023 · 7 comments
Assignees

Comments

@jooola
Copy link

jooola commented Jul 29, 2023

Initial Checks

  • I confirm that I'm using Pydantic V2

Description

I rely on the fact that my URLs don't have any trailing slashes, so I had a validator that stripped the trailing slash in pydantic v1.

In pydantic v2, the AnyHttpUrl type always appends a slash to the URL even if none is provided.

Example Code

from pydantic import AnyHttpUrl

assert str(AnyHttpUrl("http://example.org")) == "http://example.org" # Fails
assert str(AnyHttpUrl("http://example.org")) == "http://example.org/" # Succeed

Python, Pydantic & OS Version

pydantic version: 2.1.1
        pydantic-core version: 2.4.0
          pydantic-core build: profile=release pgo=true mimalloc=true
                 install path: /home/jo/code/github.com/libretime/libretime/shared/.venv/lib/python3.11/site-packages/pydantic
               python version: 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0]
                     platform: Linux-6.1.0-10-amd64-x86_64-with-glibc2.36
     optional deps. installed: ['typing-extensions']

Selected Assignee: @adriangb

@jooola jooola added bug V2 Bug related to Pydantic V2 unconfirmed Bug not yet confirmed as valid/applicable labels Jul 29, 2023
@Lawouach
Copy link

Lawouach commented Aug 1, 2023

I see the same issue as well. For now my workaround is an ugly hack to do .rstrip("/").

@samuelcolvin
Copy link
Member

I see the same issue as well. For now my workaround is an ugly hack to do .rstrip("/").

I think this is probably the right solution.

Pydantic V2 uses the rust url crate for URL validation which followed RFCs pretty religiouly.

@Lawouach
Copy link

Lawouach commented Aug 1, 2023

That might be worth documenting clearly though because as it stands it does break from past behaviour.

@Kastakin
Copy link

Kastakin commented Aug 2, 2023

I can confirm this behavior. My CORS origins did not like the change 😅

I agree that if this is the intended result of HttpUrl it should be documented in the migration guide. It took a lot of trial and error to find the root cause since I looked in the documentation but I could not find any pointer.

@adriangb
Copy link
Member

adriangb commented Aug 8, 2023

I agree a docs update would be nice. Do either of you want to make a PR to get the contribution? Otherwise, I'll make one in the next couple of days.

Interestingly I think #6996 would solve this: Annotated[str, Validate(AnyHttpUrl).transform(lambda x: str(x).strip("/"))]

@tworedz
Copy link

tworedz commented Aug 28, 2023

@adriangb If we cannot do nothing with rust crate, maybe I can add it to the migration guide and find a "right" way to solve this? (also relates to #7186 as duplicate)

Already found, that url-related migration changes located here:

pydantic/docs/migration.md

Lines 750 to 757 in ce51f5e

### Url and Dsn types in `pydantic.networks` no longer inherit from `str`
In Pydantic V1 the [`AnyUrl`][pydantic.networks.AnyUrl] type inherited from `str`, and all the other
`Url` and `Dsn` types inherited from these. In Pydantic V2 these types are built on two new `Url` and `MultiHostUrl`
classes using `Annotated`.
Inheriting from `str` had upsides and downsides, and for V2 we decided it would be better to remove this. To use these
types in APIs which expect `str` you'll now need to convert them (with `str(url)`).

So I'd like to change it like so:

diff --git a/docs/migration.md b/docs/migration.md
index c491b366..d452e417 100644
--- a/docs/migration.md
+++ b/docs/migration.md
@@ -756,6 +756,20 @@ classes using `Annotated`.
 Inheriting from `str` had upsides and downsides, and for V2 we decided it would be better to remove this. To use these
 types in APIs which expect `str` you'll now need to convert them (with `str(url)`).
 
+Pydantic V2 uses Rust's [Url](https://crates.io/crates/url) crate for URL validation, and it handles urls a little
+differently. It appends slashes to the end of the url (if it is without path), even if it is not specified in the 
+original string. For example:
+
+```py
+from pydantic import AnyUrl
+
+
+assert str(AnyUrl(url="https://google.com")) == "https://google.com/"
+assert str(AnyUrl(url="https://google.com/")) == "https://google.com/"
+assert str(AnyUrl(url="https://google.com/api")) == "https://google.com/api"
+assert str(AnyUrl(url="https://google.com/api/")) == "https://google.com/api/"
+```
+
 ### Constrained types
 
 The `Constrained*` classes were _removed_, and you should replace them by `Annotated[<type>, Field(...)]`, for example:

Also want to show the solution in the docs, but I'm not 100% sure which one is preferred. Maybe we can add Chain or this to the pydantic sources and use them later?

Because very simple MyUrl = Annotated[AnyUrl, AfterValidator(lambda x: str(x).rstrip("/"))] solution messes with types

Thanks in advance!

@sydney-runkle sydney-runkle added documentation and removed unconfirmed Bug not yet confirmed as valid/applicable bug V2 Bug related to Pydantic V2 labels Sep 28, 2023
@sydney-runkle sydney-runkle self-assigned this Sep 28, 2023
@sydney-runkle
Copy link
Member

Closing this based on the docs updates being added, but I'll leave #7186 open so that relevant discussion could be continued there (potentially re @adriangb's potential workaround with the suggested Validator class) 😄.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants