php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #67351 copy() should handle HTTP 304 response
Submitted: 2014-05-28 02:23 UTC Modified: 2020-09-29 14:21 UTC
Votes:3
Avg. Score:4.3 ± 0.9
Reproduced:2 of 2 (100.0%)
Same Version:0 (0.0%)
Same OS:0 (0.0%)
From: Andy_Schmidt at HM-Software dot com Assigned:
Status: Verified Package: Streams related
PHP Version: 5.4.28 OS: Windows 2012
Private report: No CVE-ID: None
 [2014-05-28 02:23 UTC] Andy_Schmidt at HM-Software dot com
Description:
------------
Using the "If-Modified-Since" header is desirable when copying from a remote web server, to avoid unnecessarily copying unmodified files across the Internet.

However, the copy() function does not handle a HTTP 304 response, which indicates that the file on the web server is NOT newer than the local file. It ignores the response code, uses the (empty) content from the HTTP response and overwrites the existing local file with a zero length file. This is not a useful course of action.

Since the copy() function DOES handle other 3xx return codes appropriately (such as following permanent and temporary redirects), it would make particular sense to also handle 304.

Test script:
---------------
<?php
// $fURI: 		URL to a file located on a web server
// $target_file:	Path to a local file	

$arrRequestHeaders = array(
	'http'=>array(
		'method'		=>'GET',
		'protocol_version'	=>1.1,
		'follow_location'	=>1,
		'header'		=>'If-Modified-Since: '.date( 'r', filemtime( $target_file ) )."\r\n"
			)
		);
$rc = copy( $fURI, $target_file, stream_context_create($arrRequestHeaders) );
?>

Expected result:
----------------
When a HTTP 304 response is returned, the copy() function must NOT touch the target file. copy() could return a FALSE (indicating that nothing was copied), and the script could check the HTTP response to decide on any specialized processing.

Actual result:
--------------
When a HTTP 304 response is returned, the copy() function returns TRUE and overrides the target file with a 0 length file.

Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-09-07 13:17 UTC] cmb@php.net
-Status: Open +Status: Feedback -Assigned To: +Assigned To: cmb
 [2020-09-07 13:17 UTC] cmb@php.net
I agree that the current handling of 304 Not Modified doesn't make
much sense, but I wonder how that should be handled.  Just do
nothing, but return true?  And how to handle file_get_contents()
etc.?
 [2020-09-08 00:09 UTC] Andy_Schmidt at HM-Software dot com
-Status: Feedback +Status: Assigned
 [2020-09-08 00:09 UTC] Andy_Schmidt at HM-Software dot com
Sorry - I thought I had already posted this:

Expected result:
----------------
When a HTTP 304 response is returned, the copy() function must NOT touch the target file. 
Suggestion: copy() could return a FALSE (indicating that nothing was copied), in case the script wishes to check the HTTP response to decide on any specialized processing, if any.


As far as your follow-up question regarding file_get_contents(): The HTTP 304 response can only possibly occur because the caller had explicitly/intentionally produced an "If-Modified-Since" header. Consequently, I feel it is unlikely to break existing code, if file_get_contents() were to return FALSE rather than an empty string.  This implies to the caller correctly that NO data was returned, rather than falsely implying that the file has NEW data and the data is an empty file. 
When the caller receives FALSE to an "If-Modified-Since", then their logic has the choice how to handle the FALSE condition (e.g., by short circuiting non-applicable code blocks if there is no new data.)
 [2020-09-08 15:22 UTC] cmb@php.net
-Status: Assigned +Status: Verified
 [2020-09-08 15:22 UTC] cmb@php.net
> Sorry - I thought I had already posted this:

You had.  Sorry.

Anyhow, the behavior of copy() and file_get_contents() are clearly
in error here, and not copying/fetching but returning false seems
appropriate.
 [2020-09-21 14:06 UTC] cmb@php.net
The following pull request has been associated:

Patch Name: Fix #67351: copy() should handle HTTP 304 response
On GitHub:  https://github.com/php/php-src/pull/6177
Patch:      https://github.com/php/php-src/pull/6177.patch
 [2020-09-29 14:21 UTC] cmb@php.net
-Assigned To: cmb +Assigned To:
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 14:01:29 2024 UTC