Peter-Schorn / SpotifyAPI

A Swift library for the Spotify web API. Supports all endpoints.
https://peter-schorn.github.io/SpotifyAPI/documentation/spotifywebapi
MIT License
251 stars 32 forks source link

Issue with URI Parsing #42

Closed maxwellray closed 1 year ago

maxwellray commented 1 year ago

https://github.com/Peter-Schorn/SpotifyAPI/blob/cec52a729009de9a852f9ecdb1f9f501bb0ecf24/Sources/SpotifyWebAPI/Other/SpotifyIdentifier.swift#L207

It looks like there's an issue with URI parsing. My display name (maxwell_ray) is parsed as maxwell

Peter-Schorn commented 1 year ago

Your display name is not a URI. There is a user URI associated with your account that is separate from your display name. See SpotifyUser.

maxwellray commented 1 year ago

So I call the following

let currentUser = try await spotify.api.currentUserProfile().async()
print(currentUser.uri)
let playlist = try await spotify.api.createPlaylist(
        for: currentUser.uri,
        PlaylistDetails(
                name: playlistName,
                isPublic: playlistPublic,
                isCollaborative: playlistCollaborative,
                description: plan.description
        )
).async()

and the print shows the URI as spotify:user:maxwell_ray. Jumping into the createPlaylist method, I'm able to see that userId here is parsed as maxwell, when it should be maxwell_ray

Peter-Schorn commented 1 year ago

You're right: this is a bug. Thanks for bringing this to my attention. I will update the code for parsing URIs to ensure that it correctly handles underscores, as well as any other valid characters in user ids.

Peter-Schorn commented 1 year ago

I just pushed a commit that should fix this: efd1ecd4dd9ec9d3dfb12356a8917464da84664b. When I've verified that the tests are passing, I'll tag a new version.

Peter-Schorn commented 1 year ago

Fixed in 2.2.1.

maxwellray commented 1 year ago

awesome, thanks so much! really liking the library

bbauman1 commented 1 year ago

Hey @Peter-Schorn I've gotten 2 users telling me that they're getting 403: You cannot create a playlist for another user. The majority of these were solved with the above fix, but I think there are some special cases of user URI which are causing failures. I've gotten their URI through debugging and each contain a character that stands out to me.

One contains a ! and the other contains a ë

My guesses so far are that this is either throwing off the regex parsing (doesn't seem to be the case) or that there is some percent encoding happening later on which isn't being handled properly. I'm going to fork the repo and attempt a fix which I'll open up a PR for if I figure it out, but thought I'd put it on your radar and see if you have any ideas.

Peter-Schorn commented 1 year ago

Thanks for bringing this to my attention. Here's where all URIs are parsed: https://github.com/Peter-Schorn/SpotifyAPI/blob/49839bf4025ad43cf499999193fa21e57a2ea29b/Sources/SpotifyWebAPI/Other/SpotifyIdentifier.swift#L211 But it's unlikely that ! and ë are throwing off the regex parsing: https://regex101.com/r/VQtmSC/1.

Also successfully tested with these characters in a swift project with my library:

let userIDString = "spotify:user:!ë"

let user = try! SpotifyIdentifier(uri: userIDString)
print("user: \(user)")
// user: SpotifyIdentifier(id: "!ë", idCategory: SpotifyWebAPI.IDCategory.user)
bbauman1 commented 1 year ago

Yeah thanks I tried that too. I'm going to fork and add percent encoding on the userId, then send a user a testflight build. Will update

bbauman1 commented 1 year ago

Okay @Peter-Schorn figured it out. The user uri is actually already percent encoded which is what is causing the failure. I changed my code that calls createPlaylist from

createPlaylist(for: uri, playlistDetails)

to

createPlaylist(for: uri.removingPercentEncoding!, playlistDetails)

which fixed the bug for an account with a uri containing the ë character.

Not sure what other api's this might be affecting. I assume any api that involves creating/following/modifying something on a user account

Peter-Schorn commented 1 year ago

A double percent-encoded string would definitely cause the URI to be parsed incorrectly, leading to, of course, 403: You cannot create a playlist for another. I will update the documentation to clarify that my library handles the percent-encoding of strings and that they cannot already be percent-encoded.

bbauman1 commented 1 year ago

Just to make sure we're on the same page, I'm not adding any percent encoding.

With an example userURI like abcdë

currentUserProfile()
    .map(\.uri)
    .flatMap { createPlaylist(uri: $0, playlistDetails) } 

This will fail - adding removePercentEncoding on the uri fixes it. If I print the uri output from currentUserProfile it looks something like abcd%205 (not exactly what it looks like, but whatever the percent encoded value is for ë)

Peter-Schorn commented 1 year ago

This is, indeed, a deceptively complex issue. I'll have to wait until I have more time to investigate. In the meantime, it would be interesting to know exactly how these non-ascii characters are percent-encoded.

bbauman1 commented 1 year ago

I somehow found this blog post from 2007 by @rsms (early Spotify employee) - https://rsms.me/archive/2007/an-introduction-to-spotify-uris

cmd+f for "unicode" and you'll see a part verifying this behavior on Spotify's end. The example is for the uri artist:häxor and shows that the expected uri is artist:h%C3%A4xor. It's weird that sending that value back to them fails, I wonder if it is becoming double percent encoded down the line before the request is made.

Peter-Schorn commented 1 year ago

Can you please provide the URI of a real user with non-ascii characters? I created an account with non-ascii characters in the display name, but was given a URI with only ascii characters.

bbauman1 commented 1 year ago

gaëtan_stz

Copied profile link from desktop app - https://open.spotify.com/user/ga%C3%ABtan_stz?si=4a663b95c5234bab

Peter-Schorn commented 1 year ago

I sincerely apologize for the long delay. I have just begun to investigate this issue in depth.

The example is for the uri artist:häxor and shows that the expected uri is artist:h%C3%A4xor.

You seem to have misinterpreted this blog post. häxor is percent-encoded as h%C3%A4xor in the search URI spotify:search:artist:h%C3%A4xor because this URI references a search query for an artist with the name häxor (there could be more than one artist with the same name).

The URI for the artist häxor (there's only one artist with this name) is spotify:artist:5PslwtGOc2UP3hozHrcGM0, as evidenced by the link you get by clicking on the three dots, then "Share" > "Copy link to artist" on the artist profile page (on both the desktop app and the browser):

https://open.spotify.com/artist/5PslwtGOc2UP3hozHrcGM0?si=_nmzrnzrSPitodXYXEKIzg

The last path component is always the id and the second to last path component is the id category. And a Spotify URI is (usually) of the form spotify:<ID category>:<ID>. See Spotify URIs and IDs.

Furthermore a GET request to https://api.spotify.com/v1/artists/5PslwtGOc2UP3hozHrcGM0, where the last path component should be the artist ID (see Get an Artist), returns:

{
    "external_urls": {
        "spotify": "https://open.spotify.com/artist/5PslwtGOc2UP3hozHrcGM0"
    },
    "followers": {
        "href": null,
        "total": 8
    },
    "genres": [],
    "href": "https://api.spotify.com/v1/artists/5PslwtGOc2UP3hozHrcGM0",
    "id": "5PslwtGOc2UP3hozHrcGM0",
    "images": [
        {
            "height": 640,
            "url": "https://i.scdn.co/image/ab67616d0000b273389dd91e733385a58d692104",
            "width": 640
        },
        {
            "height": 300,
            "url": "https://i.scdn.co/image/ab67616d00001e02389dd91e733385a58d692104",
            "width": 300
        },
        {
            "height": 64,
            "url": "https://i.scdn.co/image/ab67616d00004851389dd91e733385a58d692104",
            "width": 64
        }
    ],
    "name": "Häxor",
    "popularity": 0,
    "type": "artist",
    "uri": "spotify:artist:5PslwtGOc2UP3hozHrcGM0"
}
Peter-Schorn commented 1 year ago
currentUserProfile()
    .map(\.uri)
    .flatMap { createPlaylist(uri: $0, playlistDetails) } 

The reason why this fails is, indeed, because the user URI is being double percent-encoded. SpotifyUser.uri (the type returned by SpotifyAPI.currentUserProfile) is already percent-encoded and the URI passed in to SpotifyAPI.createPlaylist is percent-encoded again. In fact, every value passed in to any SpotifyAPI method that appears in the resulting URL is percent-encoded.

I think the solution is for my code to percent-decode the URI in the JSON payload for SpotifyUser, meaning that SpotifyUser.uri will not be percent-encoded. I'm working on that now.

Peter-Schorn commented 1 year ago

Fixed in 2.2.3.

rsms commented 1 year ago

Hi Peter :-) 👋