You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Nicholas Telford (JIRA)" <ji...@apache.org> on 2010/02/25 18:02:28 UTC

[jira] Created: (THRIFT-718) Thrift PHP library includes closing tags and extraneous whitespace

Thrift PHP library includes closing tags and extraneous whitespace
------------------------------------------------------------------

                 Key: THRIFT-718
                 URL: https://issues.apache.org/jira/browse/THRIFT-718
             Project: Thrift
          Issue Type: Improvement
          Components: Library (PHP)
            Reporter: Nicholas Telford


Extraneous whitespace after a PHP script's closing tag (?>) can cause big problems when sending headers to the client.

It is generally advised that closing PHP tags be omitted from the end of a PHP file to prevent extraneous whitespace being an issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-718) Thrift PHP library includes closing tags and extraneous whitespace

Posted by "Nicholas Telford (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12839663#action_12839663 ] 

Nicholas Telford commented on THRIFT-718:
-----------------------------------------

Didn't realise I'd mixed the line-endings, I'll try to fix those in the patch.

Closing tags on their own are fine, but can potentially cause problems because *any* characters afterwards will cause output to be sent to the browser (unless output buffering is enabled) before headers are sent - cue PHP WARNING.

In the case of the Thrift interface, it seems you've intentially added characters (yes, by whitespace I meant the LFs) after the closing tags. This illustrates why closing tags are dangerous.

Omitting the closing tags allows you to add as much whitespace to the end of the file as you like, because PHP will never output them.

This is an official part of the Zend Framework coding standards, and I believe other frameworks and libraries have also adopted the practice. More details here: http://framework.zend.com/manual/en/coding-standard.php-file-formatting.html

PEAR is a bit of a mess, I wouldn't trust it implicitly.

> Thrift PHP library includes closing tags and extraneous whitespace
> ------------------------------------------------------------------
>
>                 Key: THRIFT-718
>                 URL: https://issues.apache.org/jira/browse/THRIFT-718
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (PHP)
>            Reporter: Nicholas Telford
>         Attachments: thrift_php_closingTags.diff
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> Extraneous whitespace after a PHP script's closing tag (?>) can cause big problems when sending headers to the client.
> It is generally advised that closing PHP tags be omitted from the end of a PHP file to prevent extraneous whitespace being an issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (THRIFT-718) Thrift PHP library includes closing tags and extraneous whitespace

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-718?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury closed THRIFT-718.
--------------------------------

         Assignee: Nicholas Telford
    Fix Version/s: 0.4
       Resolution: Fixed

I fixed the newlines at EOF and committed this patch. Thanks Nick!

> Thrift PHP library includes closing tags and extraneous whitespace
> ------------------------------------------------------------------
>
>                 Key: THRIFT-718
>                 URL: https://issues.apache.org/jira/browse/THRIFT-718
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (PHP)
>            Reporter: Nicholas Telford
>            Assignee: Nicholas Telford
>             Fix For: 0.4
>
>         Attachments: thrift_php_closingTags.diff, thrift_php_closingTags2.diff
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> Extraneous whitespace after a PHP script's closing tag (?>) can cause big problems when sending headers to the client.
> It is generally advised that closing PHP tags be omitted from the end of a PHP file to prevent extraneous whitespace being an issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-718) Thrift PHP library includes closing tags and extraneous whitespace

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12839777#action_12839777 ] 

David Reiss commented on THRIFT-718:
------------------------------------

bq. Closing tags on their own are fine, but can potentially cause problems because any characters afterwards will cause output to be sent to the browser (unless output buffering is enabled) before headers are sent - cue PHP WARNING.
bq. In the case of the Thrift interface, it seems you've intentially added characters (yes, by whitespace I meant the LFs) after the closing tags. This illustrates why closing tags are dangerous.
Are you under the impression that the Thrift libraries are currently outputting newlines?  They are not.

I find the Zend guideline link compelling.  Mark, Chris G, what do you think?

Please resubmit your patch with newlines at EOF.

> Thrift PHP library includes closing tags and extraneous whitespace
> ------------------------------------------------------------------
>
>                 Key: THRIFT-718
>                 URL: https://issues.apache.org/jira/browse/THRIFT-718
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (PHP)
>            Reporter: Nicholas Telford
>         Attachments: thrift_php_closingTags.diff, thrift_php_closingTags2.diff
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> Extraneous whitespace after a PHP script's closing tag (?>) can cause big problems when sending headers to the client.
> It is generally advised that closing PHP tags be omitted from the end of a PHP file to prevent extraneous whitespace being an issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-718) Thrift PHP library includes closing tags and extraneous whitespace

Posted by "Nicholas Telford (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-718?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nicholas Telford updated THRIFT-718:
------------------------------------

    Attachment: thrift_php_closingTags2.diff

Patch without CR chars.

I don't know how the hell my diff program decided to add seemingly random CR chars, but I've removed them now. Shall be more vigilant in future.

> Thrift PHP library includes closing tags and extraneous whitespace
> ------------------------------------------------------------------
>
>                 Key: THRIFT-718
>                 URL: https://issues.apache.org/jira/browse/THRIFT-718
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (PHP)
>            Reporter: Nicholas Telford
>         Attachments: thrift_php_closingTags.diff, thrift_php_closingTags2.diff
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> Extraneous whitespace after a PHP script's closing tag (?>) can cause big problems when sending headers to the client.
> It is generally advised that closing PHP tags be omitted from the end of a PHP file to prevent extraneous whitespace being an issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-718) Thrift PHP library includes closing tags and extraneous whitespace

Posted by "Nicholas Telford (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-718?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nicholas Telford updated THRIFT-718:
------------------------------------

    Attachment: thrift_php_closingTags.diff

Patch to remove closing tags and extraneous whitespace from Thrift PHP library.

> Thrift PHP library includes closing tags and extraneous whitespace
> ------------------------------------------------------------------
>
>                 Key: THRIFT-718
>                 URL: https://issues.apache.org/jira/browse/THRIFT-718
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (PHP)
>            Reporter: Nicholas Telford
>         Attachments: thrift_php_closingTags.diff
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> Extraneous whitespace after a PHP script's closing tag (?>) can cause big problems when sending headers to the client.
> It is generally advised that closing PHP tags be omitted from the end of a PHP file to prevent extraneous whitespace being an issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-718) Thrift PHP library includes closing tags and extraneous whitespace

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12838471#action_12838471 ] 

David Reiss commented on THRIFT-718:
------------------------------------

Your patch mixes LF and CRLF line endings.  Please remove the CRs.

I just looked at two random packages from PEAR, and both had closing tags at the end.

Also, I don't see any extraneous whitespace.  Do you mean the newlines at end-of-file?  Those are supposed to be there.

> Thrift PHP library includes closing tags and extraneous whitespace
> ------------------------------------------------------------------
>
>                 Key: THRIFT-718
>                 URL: https://issues.apache.org/jira/browse/THRIFT-718
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Library (PHP)
>            Reporter: Nicholas Telford
>         Attachments: thrift_php_closingTags.diff
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> Extraneous whitespace after a PHP script's closing tag (?>) can cause big problems when sending headers to the client.
> It is generally advised that closing PHP tags be omitted from the end of a PHP file to prevent extraneous whitespace being an issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.