Fix search with non-ASCII terms by removing manual '+' substitution#25
Open
ParkJeongseop wants to merge 1 commit into
Open
Fix search with non-ASCII terms by removing manual '+' substitution#25ParkJeongseop wants to merge 1 commit into
ParkJeongseop wants to merge 1 commit into
Conversation
Pre-replacing spaces with '+' conflicted with requests' percent- encoding for non-ASCII terms, so the Apple Music server received a broken query and returned unrelated results.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fix search with non-ASCII terms by removing manual
+substitutionSummary
AppleMusic.search()silently returns unrelated results when the term contains non-ASCII characters (Korean, Japanese, Chinese, etc.). Root cause is a pre-processing step that conflicts with howrequestsencodes query parameters. This PR removes the broken pre-processing and letsrequestshandle URL encoding directly, fixing searches for all languages. The now-unusedosparameter ('linux'/'windows') is removed as well, since both branches were workarounds for the same underlying bug.What changed
term = re.sub(' +', '+', term)fromsearch()and letrequestshandle URL encoding end-to-end. The pre-substitution conflicted withrequests' percent-encoding for non-ASCII terms:+got re-encoded to%2B, the server decoded it back as a literal+, the query matched nothing, and the API silently fell back to returning trending results with200 OK.osparameter removed — Theos='linux'/os='windows'branches were a workaround for the same bug above, not a real OS-compatibility layer. The name was misleading: behavior was determined entirely by whether the term contained non-ASCII characters, not by the operating system. With the pre-substitution gone, both branches become identical, so the parameter has no remaining purpose.import reinclient.py, and removedtest_search_windowsintests.py(it exercised the deletedos='windows'path).Backward compatibility
AppleMusic.search()no longer accepts theoskeyword argument. Callers passingos='linux'oros='windows'will see:Mitigation is to simply drop the argument — the default path now works correctly in every environment and for every language, so no alternative is needed.
No other public API changes.
song(),album(),songs_by_isrc(), etc. are untouched.Verification
Reproduction against live Apple Music API (Korean phrase "Landing in Love"):
Before:
After:
Same symptom reproduces with any non-ASCII term (e.g.
'恋をしたのは','愛してる'). ASCII terms "worked" before only becauserequestshappened to leave+intact in those cases, which the server then interpreted as a space — effectively masking the underlying bug for English searches.Test plan
test_searchpasses'사랑하게 될 거야'(storefront='kr',l='ko-KR') returns correctly matched tracks against the live API'IU'returns expected tracks — no regressionsongs_by_isrc(['USUM71410382'])returns the expected three Adam Levine tracks — no regression (songs_by_isrcuses a different endpoint, included as a sanity check that the broader client is unaffected)