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

dns: don't respond with FORMERR if message has ID 0 #35129

Merged
merged 6 commits into from
Jul 22, 2024

Conversation

michaelbeaumont
Copy link
Contributor

Commit Message:
dns: don't respond with FORMERR if message has ID 0

Additional Description:
0 is a valid ID according to RFC 1035

Risk Level:
Low

Testing:
./ci/run_envoy_docker.sh './ci/do_ci.sh dev'

Docs Changes:
N/A

Release Notes:
dns: don't respond with FORMERR if message has ID 0

Platform Specific Features: N/A

Fixes #35097

Copy link

Hi @michaelbeaumont, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #35129 was opened by michaelbeaumont.

see: more, trace.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Can you add a release note for this one? Otherwise LGTM, thanks.

/wait

@yanjunxiang-google
Copy link
Contributor

LGTM modulo open comments and CI. Thanks for fixing this.

@michaelbeaumont
Copy link
Contributor Author

Could the current CI failures be unrelated to this change? Having a bit of trouble running them locally but they seem quite unlikely to be caused by this

@yanjunxiang-google
Copy link
Contributor

/retest

@lahabana
Copy link

/backport

@michaelbeaumont
Copy link
Contributor Author

michaelbeaumont commented Jul 22, 2024

@yanjunxiang-google @mattklein123 anything still needed for this? it's starting to run into conflicts with main

Oh I just noticed this missed v1.31 :/

@mattklein123 mattklein123 enabled auto-merge (squash) July 22, 2024 15:15
@mattklein123 mattklein123 merged commit b8ae3ab into envoyproxy:main Jul 22, 2024
49 of 50 checks passed
@michaelbeaumont michaelbeaumont deleted the fix/dns-id-0-v1.27 branch July 22, 2024 16:54
martinduke pushed a commit to martinduke/envoy that referenced this pull request Aug 8, 2024
Commit Message:
dns: don't respond with FORMERR if message has ID 0

Additional Description:
0 is a valid ID according to RFC 1035

Risk Level:
Low

Testing:
`./ci/run_envoy_docker.sh './ci/do_ci.sh dev'`

Docs Changes:
N/A

Release Notes:
dns: don't respond with FORMERR if message has ID 0

Platform Specific Features: N/A

Fixes envoyproxy#35097

Signed-off-by: Mike Beaumont <[email protected]>
Signed-off-by: Martin Duke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/review Request to backport to stable releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Envoy's DNS filter fails to handle DNS messages with an ID equal to 0
5 participants