search

spandex-project / spandex_phoenix

Phoenix Instrumentation tracer
MIT License
82 stars 29 forks source link

[SC-562] fix: do not fail tracer for malformed uri #49

Open mpmiszczyk opened 3 years ago

mpmiszczyk commented 3 years ago

Failing tracer will be detached which can brake tracing all together.

sourcelevel-bot[bot] commented 3 years ago

Hello, @mpmiszczyk! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

GregMefford commented 3 years ago

Hey! Thanks so much for taking the time to contribute this PR! ❤️ 🚀

I am ready to merge it essentially as it is, but there are some minor merge conflicts and I don't have permission to push the rebased changes to your branch. Could you please do a rebase onto master to get it up to date? There's just a test that was removed in a previous PR that you'll need to also remove when you resolve the conflicts in that file.

Alternatively, if you see the option to allow maintainers to push to your branch, you can turn that on and I can update it myself.

Sija commented 2 years ago

I'm having a difficulty in understanding why this (PR) would be needed at all.

  1. URI.decode shouldn't raise
  2. auth%%27%20AND%202*3*8=6*8%20AND%20%27zPT3%27!=%27zPT3% is not no an URI, but a a path, and still URI.decode wouldn't raise given this as an argument
  3. Invalid paths should be passed along without alteration, otherwise we're losing important information