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

Add WordPress.Security.SafeRedirect XML documentation #1742

Conversation

marconmartins
Copy link
Contributor

No description provided.

@jrfnl jrfnl added this to In progress in XML Documentation Jun 23, 2019
XML Documentation automation moved this from In progress to Review in progress Jun 23, 2019
<documentation title="Safe Redirect">
<standard>
<![CDATA[
Safe redirects must be used.
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see this expanded - why must safe redirects be used? What is a safe redirect? Why does a non-safe redirect even exist? Where can I find out more about this concept? etc.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Hi @marconmartins, thank you again for this PR. Mostly looking great.

I do concur with @GaryJones that it would be great if the generic rule explanation could be expanded a bit.

The following articles may help to get some inspiration:

Other than that, the docs are just missing the <em> ...</em> tags to highlight good/bad code, but we weren't that clear about that in the initial explanation, so sorry about that!

I look forward to the next iteration!

<code_comparison>
<code title="Valid: Safe redirect.">
<![CDATA[
wp_safe_redirect( $location );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
wp_safe_redirect( $location );
<em>wp_safe_redirect</em>( $location );

</code>
<code title="Invalid: Unsafe redirect.">
<![CDATA[
wp_redirect( $location );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
wp_redirect( $location );
<em>wp_redirect</em>( $location );

@jrfnl
Copy link
Member

jrfnl commented Oct 7, 2019

@marconmartins Just wondering if you'll have a chance to finish this off in the near future.
I'd like to release WPCS 2.2.0 soon and it would be great if this PR could be included.

If you haven't got time or lost interest, please let us know and we'll see if we can find someone to take over.

@jrfnl
Copy link
Member

jrfnl commented May 16, 2022

Closing as fixed via #1826 which was merged quite a while ago.

@jrfnl jrfnl closed this May 16, 2022
XML Documentation automation moved this from Review in progress to Done May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants