Skip to content

fix(client): URL encoding bug #1

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arthurgousset
Copy link
Member

@arthurgousset arthurgousset commented May 21, 2025

Description

Fixes meilisearch#511, although with a nuance. The bug in meilisearch#511 (root cause analysis for reference) is already fixed from a user-perspective. Not because the Meilisearch SDK explicitly encodes URLs, but because reqwest (the HTTP client) appears to automatically encode URLs when they're passed to it.

Changes:

  1. fixes URL encoding in src/indexes.rs
  2. adds reproduction tests (that can be omitted from the PR before merging)

Bug Description

This bug relates to a problem where documents with single quotes in their primary key field silently failed to be added to the Meilisearch index due to improper URL handling in the Rust SDK. Specifically, the primary key parameter was directly interpolated into the URL without URL encoding, causing malformed URLs when special characters were present.

Steps to Reproduce

  1. Checked out the Meilisearch Rust SDK (version 0.28.0)
  2. Created a test file with a test that:
    • Sets up a mock server using mockito
    • Creates mocks for both encoded and unencoded URLs with primary key containing a single quote
    • Makes a request to add a document with a primary key containing a single quote
    • Checks which URL format is being used (encoded or unencoded)
  3. Ran the test with cargo test -- tests::index_primary_key_test_version_check::test_reqwest_handles_url_encoding --nocapture

Expected vs Actual Behavior

Expected Behavior (based on RCA)

  • The URL passed to the HTTP client should not be properly encoded
  • Special characters like single quotes in the primary key should cause malformed URLs
  • API requests with primary keys containing special characters should fail

Actual Behavior

  • The URL is automatically encoded by the reqwest HTTP client
  • Special characters like single quotes are properly URL-encoded (e.g., ' → %27)
  • Requests with primary keys containing special characters work correctly

Important Findings

  1. The code in src/indexes.rs still directly interpolates the primary key parameter without encoding:
    format!(
        "{}/indexes/{}/documents?primaryKey={}",
        self.client.host, self.uid, primary_key
    )
  2. However, reqwest (which is now the default HTTP client in version 0.28.0) automatically encodes URLs when making requests
  3. This means the bug has been indirectly fixed by switching the HTTP client to one that handles URL encoding automatically
  4. The bug would likely still exist in older versions of the SDK that used different HTTP clients

This explains why the code appears to still have the bug (no explicit encoding) but actually works correctly (thanks to reqwest's automatic URL encoding).

Other changes

NA

Tested

Root cause: https://hyperdrive.engineering/#report-49eaa957-fe2c-44f4-a9f0-76507540bd54

cargo test -- tests::index_primary_key_test_version_check::test_reqwest_handles_url_encoding --nocapture
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.32s
     Running unittests src/lib.rs (target/debug/deps/meilisearch_sdk-cd49669b8642c05f)

running 1 test
Current version: 0.28.0
Checking if reqwest handles URL encoding automatically...
Properly encoded URL matched: true
Unencoded URL matched: false

CONCLUSION:
Bug fix: The bug described in the RCA appears to be fixed, not because the SDK explicitly encodes URLs,
but because reqwest (the HTTP client) appears to automatically encode URLs when they're passed to it.
test tests::index_primary_key_test_version_check::test_reqwest_handles_url_encoding ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 134 filtered out; finished in 0.03s

   Doc-tests meilisearch_sdk

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 176 filtered out; finished in 0.01s
cargo test -- tests::index_primary_key_test::test_primary_key_with_single_quote --nocapture
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.24s
     Running unittests src/lib.rs (target/debug/deps/meilisearch_sdk-cd49669b8642c05f)

running 1 test
Encoded URL matched: true
Unencoded URL matched: false
test tests::index_primary_key_test::test_primary_key_with_single_quote ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 134 filtered out; finished in 0.03s

   Doc-tests meilisearch_sdk

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 176 filtered out; finished in 0.01s

Related issues

Backwards compatibility

Yes.

Documentation

NA

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

Successfully merging this pull request may close these issues.

Quotation marks in primary silently fails entire index.add_document call.
1 participant