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 EZP-20289 Symfony CSRF protection not integrated with legacy #211

Merged
merged 2 commits into from
Jan 30, 2013

Conversation

andrerom
Copy link
Contributor

This is the Symfony side of things injecting csrf settings if csrf is enabled
into api added in ezsystems/ezpublish-legacy#552

This is the Symfony side of things injecting csrf settings if csrf is enabled
into api added in ezsystems/ezpublish-legacy#552
@@ -90,6 +97,14 @@ public function onBuildKernel( PreBuildKernelEvent $event )
$settings + (array)$event->getParameters()->get( "injected-settings" )
);

// Inject csrf protection settings to make sure legacy & symfony stack work together
if ( $this->container->hasParameter( 'form.type_extension.csrf.enabled' ) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

CS : Shouldn't the first condition be on the following line ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, the phpcs profile didn't complain so I just coded it like I'm used to from 4.x cs.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think, @patrickallaert ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes @lolautruche, normally ending parenthesis and brackets should be aligned on the same column that the first character of the opening line. I guess the corresponding sniff is working on functions/methods and not yet on language keywords.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find any example on https://github.com/ezsystems/ezp-next/wiki/codingstandards
But I guess it is supposed to be like this?

if (
    // expression
)
{
    // code
}

This is a part of the new cs I kind of don't like because you get the ending of the parentheses on a new line :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrerom It has the merit of being very consistent. Whatever the closing is ("}])"), it must always be at the same column level than the first character of the line where it has been opened in the case it's not on the same line.
This is also very easy to program in a code sniffer rule, even if this is not the goal, and hence why that sniff already existed and is used by several standards.

@lolautruche
Copy link
Contributor

+1

@patrickallaert
Copy link
Contributor

Besides @lolautruche's remark: +1

andrerom added a commit that referenced this pull request Jan 30, 2013
Fix EZP-20289 Symfony CSRF protection not integrated with legacy
@andrerom andrerom merged commit 05a38e4 into master Jan 30, 2013
@andrerom andrerom deleted the legacy_csrf_inject branch January 30, 2013 13:01
ViniTou pushed a commit that referenced this pull request Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants