Skip to content

Use UTF-8 encoding for nginx plugin#6725

Closed
schoen wants to merge 6 commits intomasterfrom
nginx-utf8
Closed

Use UTF-8 encoding for nginx plugin#6725
schoen wants to merge 6 commits intomasterfrom
nginx-utf8

Conversation

@schoen
Copy link
Copy Markdown
Contributor

@schoen schoen commented Jan 30, 2019

Based on and supersedes #5341.

Fixes #5337.

@schoen
Copy link
Copy Markdown
Contributor Author

schoen commented Jan 30, 2019

@ohemorange, I've just merged this with the current master branch and I think @g6123 had actually satisfied all of our outstanding substantive requests on the PR. Can you think of other things that need to be changed or improved?

Copy link
Copy Markdown
Contributor

@ohemorange ohemorange left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that the last commit added some code that doesn't have a test, let's put in a test for that and then get this in.

return nginxparser.load(_file)
except IOError:
logger.warning("Missing NGINX TLS options file: %s", ssl_options)
except UnicodeDecodeError:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code needs coverage

@ohemorange
Copy link
Copy Markdown
Contributor

@schoen this PR is so close! when do you think you'll get a chance to finish it up?

@ohemorange ohemorange self-assigned this Apr 24, 2019
@ohemorange
Copy link
Copy Markdown
Contributor

Hey @schoen, any chance we could finish this up?

@ohemorange
Copy link
Copy Markdown
Contributor

Bumping on this

@g6123 g6123 mentioned this pull request Feb 23, 2020
2 tasks
@schoen schoen closed this in #7797 Mar 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nginx plugin crashes when non-ascii configuration file read

3 participants