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

[3.5] Twig exception request santitisation handling #7569

Merged
merged 1 commit into from
Jul 19, 2018
Merged

[3.5] Twig exception request santitisation handling #7569

merged 1 commit into from
Jul 19, 2018

Conversation

GwendolenLynch
Copy link
Contributor

  • Handle nested maps & indexed arrays in RequestSanitiser
  • [Tests] Broaden RequestSanitiser tests to cover more data types

Fixes #7540
Closes #7543
Closes #7175

- [Tests] Broaden RequestSanitiser tests to cover more data types
@Boorj
Copy link
Contributor

Boorj commented Jul 19, 2018

nice) +7 lines of code && +over5000 lines of tests ;) Great job, @GawainLynch !

By the way, i don't think, that it closes #7175 .. ) There was a conversation about fallback for missing 'preview' route in routing.yml

@GwendolenLynch
Copy link
Contributor Author

By the way, i don't think, that it closes 7175 .. ) There was a conversation about fallback for missing 'preview' route in routing.yml

For the purposes of tracking what I need to, it does. What is requested isn't coming to v3 for stated reasons.

Copy link
Member

@bobdenotter bobdenotter left a comment

Choose a reason for hiding this comment

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

Looks good to me, as well.

As for the edge-case @Boorj mentions: I'm also of the opinion that if you modify the routes that are marked "Edit these at your own risk". :-)

Let's :shipit:

@bobdenotter bobdenotter merged commit 308b789 into bolt:3.5 Jul 19, 2018
@GwendolenLynch GwendolenLynch deleted the hotfix/twig-exception-handling branch July 19, 2018 07:46
@Boorj
Copy link
Contributor

Boorj commented Jul 22, 2018

I'm of the opinion : if you create a refrigerator and put a bottle of poison with a sign ’you'll probably be poisoned’ in each one, be ready that someone will be poisoned. 'routing.yml' provides convenient way to define routes, and nobody loves tons of garbage there.

Predefined routes should have a fallback, with a possibility to override them in routing.yml. It's a consistent behavior, since every Extension has getDefaultConfig(), which is overridden by extension.config.yml.

But...

@bobdenotter told that Bolt 4 will be using symfony 4, and it will have another routing. So i'm ok with that.

yeah, let's shipit

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.

None yet

4 participants