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

Fix: Cannot refresh if a query name contain control characters. #5602

Conversation

iwakiriK
Copy link
Contributor

@iwakiriK iwakiriK commented Sep 27, 2021

What type of PR is this? (check all applicable)

  • Bug Fix

Description

If a query name contains control characters, it cannot be refreshed.

control characters

We find out that the problem occurs in the response header.
https://github.com/getredash/redash/blob/v10.0.0-beta/redash/handlers/query_results.py#L422

This PR will fix this problem.

How to Reproduce

Related Tickets & Documents

https://www.regular-expressions.info/unicode.html
https://www.compart.com/en/unicode/category
https://pypi.org/project/regex/
https://stackoverflow.com/questions/1832893/python-regex-matching-unicode-properties/4316097#4316097

@guidopetri
Copy link
Contributor

@iwakiriK , thanks for the PR! Same here, we've updated a lot of things now that we're Community-driven so - if you're still interested in getting this merged - would you mind rebasing off master to re-run the CI, as well as updating merge conflicts?

We're trying to clean up our PR todo list, so if you're not interested, that's fine - we'll close the PR in about a week if we don't hear back. If you're interested in reopening the PR afterwards, we would also very much welcome that.

@justinclift
Copy link
Member

Just attempted to fix the merge conflict using the web interface. It'll probably need some formatting clean up though.

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #5602 (5410b97) into master (1af49e9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5602   +/-   ##
=======================================
  Coverage   60.73%   60.74%           
=======================================
  Files         154      154           
  Lines       12626    12628    +2     
  Branches     1716     1716           
=======================================
+ Hits         7669     7671    +2     
  Misses       4732     4732           
  Partials      225      225           
Files Changed Coverage Δ
redash/handlers/query_results.py 81.57% <100.00%> (+0.19%) ⬆️

@justinclift
Copy link
Member

@konnectr Any interest in reviewing this? It's passing our CI tests, and looks fairly simple. 😄

@justinclift
Copy link
Member

Thanks heaps @konnectr. 😄

@justinclift justinclift enabled auto-merge (squash) August 23, 2023 20:02
@justinclift justinclift merged commit 5eeeb5c into getredash:master Aug 23, 2023
12 checks passed
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.

4 participants