Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Fix IPRO and missing Content-Length headers in 32-bit builds. #1196

Merged
merged 1 commit into from
May 11, 2016
Merged

Fix IPRO and missing Content-Length headers in 32-bit builds. #1196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/ngx_base_fetch.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,18 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* Author: jefftk@google.com (Jeff Kaufman)
*/

// Author: jefftk@google.com (Jeff Kaufman)
#include "ngx_pagespeed.h" // Must come first, see comments in CollectHeaders.

#include <unistd.h> //for usleep

#include "ngx_base_fetch.h"
#include "ngx_event_connection.h"
#include "ngx_list_iterator.h"

#include "ngx_pagespeed.h"

#include "net/instaweb/rewriter/public/rewrite_driver.h"
#include "net/instaweb/rewriter/public/rewrite_options.h"
#include "net/instaweb/rewriter/public/rewrite_stats.h"
Expand Down Expand Up @@ -255,6 +256,13 @@ ngx_int_t NgxBaseFetch::CollectAccumulatedWrites(ngx_chain_t** link_ptr) {
}

ngx_int_t NgxBaseFetch::CollectHeaders(ngx_http_headers_out_t* headers_out) {
// nginx defines _FILE_OFFSET_BITS to 64, which changes the size of off_t.
// If a standard header is accidentally included before the nginx header,
// on a 32-bit system off_t will be 4 bytes and we don't assign all the
// bits of content_length_n. Sanity check that did not happen.
// This could use static_assert, but this file is not built with --std=c++11.
bool sanity_check_off_t[sizeof(off_t) == 8 ? 1 : -1] __attribute__ ((unused));
Copy link
Contributor

Choose a reason for hiding this comment

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

nice


const ResponseHeaders* pagespeed_headers = response_headers();

if (content_length_known()) {
Expand Down
3 changes: 2 additions & 1 deletion src/ngx_pagespeed.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,8 @@ void copy_response_headers_from_ngx(const ngx_http_request_t* r,

// When we don't have a date header, set one with the current time.
if (headers->Lookup1(HttpAttributes::kDate) == NULL) {
headers->SetDate(ngx_current_msec);
PosixTimer timer;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we comment the hell out of this variable and its ancestors that it is totally bogus on 32-it?

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 did check that we're not using it anywhere else. I don't think a comment would be much use, though something to actually hide/break the variable wouldn't be an awful idea. Not sure exactly how to go about that, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind; I see this is just a big fat global bomb supplied by nginx that we are using.

headers->SetDate(timer.NowMs());
}

// TODO(oschaaf): ComputeCaching should be called in setupforhtml()?
Expand Down