Skip to content

PPI::Token::HereDoc->heredoc() includes the terminator for quoted terminators #71

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

Closed
wants to merge 2 commits into from
Closed

PPI::Token::HereDoc->heredoc() includes the terminator for quoted terminators #71

wants to merge 2 commits into from

Conversation

guillaumeaubert
Copy link
Contributor

While working with @vsespb on guillaumeaubert/Perl-Critic-Policy-ValuesAndExpressions-PreventSQLInjection#14, I discovered that heredoc blocks with a quoted terminator (such as <<"HERE" or <<'HERE') include the terminator line in the output of ->heredoc(), but unquoted heredoc blocks (such as <<HERE) do not.

The issue is the check for the terminator on this line https://github.com/adamkennedy/PPI/blob/master/lib/PPI/Token/HereDoc.pm#L212:

if ( $line eq $terminator ) {

$terminator is the unescaped version of the terminator without surrounding quotes, while the lines that are being compared against will have the escaped version. This also explains why it's not an issue for regular barewords, since for those the escaped and unescaped versions are the same.

I added tests in t/ppi_token_heredoc.t to cover the five possible cases in lib/PPI/Token/HereDoc.pm, and the tests pass after adding the patch in 0295784. The tests are modeled after tests in other t/ppi_token_* files for consistency.

I also opted to add a _raw_terminator key rather than modifying _terminator in the PPI::Token::HereDoc object, in case this would break code that relies on this behavior (even if the key is prefixed with an underscore to indicate that it's private). But this can easily be changed to reuse _terminator if it is preferable.

Thank you for considering this pull request!

guillaumeaubert added a commit to guillaumeaubert/Perl-Critic-Policy-ValuesAndExpressions-PreventSQLInjection that referenced this pull request Mar 19, 2014
Due to a bug in PPI::Token::Heredoc, $token->heredoc() does not include the
terminator line when the terminator is a simple bareword, but it includes the
terminator line in the other cases (single/double quoted , commands, escaped).

I wrote a patch to resolve this in PPI (see
Perl-Critic/PPI#71), but in the meantime this addresses
the inconsistency.
@guillaumeaubert
Copy link
Contributor Author

My mistake, the closing terminator should not include quotes/backticks/etc, or it will not be detected as a valid closing terminator and be included in the heredoc() output instead. So the original code is correct, and I'm closing the pull request accordingly.

@guillaumeaubert guillaumeaubert deleted the heredoc_terminator branch March 20, 2014 01:35
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.

1 participant