Skip to content

fix(store): keep quotes and escape whitespace in store key queries - #241

Open
mohsinaliwork686-rgb wants to merge 1 commit into
redis-developer:mainfrom
mohsinaliwork686-rgb:fix/store-get-quote-and-whitespace
Open

mohsinaliwork686-rgb wants to merge 1 commit into
redis-developer:mainfrom
mohsinaliwork686-rgb:fix/store-get-quote-and-whitespace

Conversation

@mohsinaliwork686-rgb

Copy link
Copy Markdown

Fixes #237

get() built its query with redisvl's Text and Tag filters. Text turns a double quote into a space, so the namespace no longer matched what put() stored. Tag leaves a newline unescaped, which ends the tag value. In both cases get() returned None for an item that search() could find.

The query now keeps the escaped namespace as it is, still as a phrase, because an unquoted @prefix: also matches child namespaces. Keys go through a new _tag_escaper, which adds | and whitespace other than a space to the default set.

The other @key:{...} queries had the same problem, so they use it too. On main, with a key containing a newline or |:

  • delete() leaves the item in place.
  • A second put() keeps the old document next to the new one.

Stored data doesn't change, so existing items stay readable.

Not covered:

  • A newline or carriage return in a namespace label. get() and search() both miss those items, and none of the query forms I tried matched them.
  • The empty key from the issue, which still raises SEARCH_SYNTAX.

This changes the same delete line as #239, so whichever merges second will need a small rebase.

Tests are in tests/test_issue_237_store_get_escaping.py: get, update and delete, sync and async, for a double quote, newline, carriage return, tab, vertical tab, form feed and |, plus a batched get and a check that get() stays within the exact namespace. 15 of the 16 fail without the change. The namespace check passes either way.

I ran these and the existing store tests against redis:8 (8.10.3) in a local container, with a small redis_url fixture, because I couldn't install testcontainers here. 90 passed and 1 failed: test_async_store_setup_on_unreachable_redis_raises_connection_error, which fails the same way without this change. black and isort pass. I didn't run the rest of the test suite or mypy locally.

This branch has not been deployed

No deployments
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.

RedisStore.get() returns None for an item that search() finds: double quote in a namespace label, newline in a key

1 participant