Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(485)

Side by Side Diff: net/http/http_stream_parser.cc

Issue 1255123006: Revert of Net: Stop treating partial HTTP headers as a valid response. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « net/http/http_network_transaction_unittest.cc ('k') | net/http/http_stream_parser_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/http/http_stream_parser.h" 5 #include "net/http/http_stream_parser.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/compiler_specific.h" 8 #include "base/compiler_specific.h"
9 #include "base/logging.h" 9 #include "base/logging.h"
10 #include "base/metrics/histogram_macros.h" 10 #include "base/metrics/histogram_macros.h"
(...skipping 11 matching lines...) Expand all
22 #include "net/socket/client_socket_handle.h" 22 #include "net/socket/client_socket_handle.h"
23 #include "net/socket/ssl_client_socket.h" 23 #include "net/socket/ssl_client_socket.h"
24 24
25 namespace net { 25 namespace net {
26 26
27 namespace { 27 namespace {
28 28
29 enum HttpHeaderParserEvent { 29 enum HttpHeaderParserEvent {
30 HEADER_PARSER_INVOKED = 0, 30 HEADER_PARSER_INVOKED = 0,
31 HEADER_HTTP_09_RESPONSE = 1, 31 HEADER_HTTP_09_RESPONSE = 1,
32 // Obsolete: HEADER_ALLOWED_TRUNCATED_HEADERS = 2, 32 HEADER_ALLOWED_TRUNCATED_HEADERS = 2,
33 HEADER_SKIPPED_WS_PREFIX = 3, 33 HEADER_SKIPPED_WS_PREFIX = 3,
34 HEADER_SKIPPED_NON_WS_PREFIX = 4, 34 HEADER_SKIPPED_NON_WS_PREFIX = 4,
35 NUM_HEADER_EVENTS 35 NUM_HEADER_EVENTS
36 }; 36 };
37 37
38 void RecordHeaderParserEvent(HttpHeaderParserEvent header_event) { 38 void RecordHeaderParserEvent(HttpHeaderParserEvent header_event) {
39 UMA_HISTOGRAM_ENUMERATION("Net.HttpHeaderParserEvent", header_event, 39 UMA_HISTOGRAM_ENUMERATION("Net.HttpHeaderParserEvent", header_event,
40 NUM_HEADER_EVENTS); 40 NUM_HEADER_EVENTS);
41 } 41 }
42 42
(...skipping 718 matching lines...) Expand 10 before | Expand all | Expand 10 after
761 761
762 return result; 762 return result;
763 } 763 }
764 764
765 int HttpStreamParser::HandleReadHeaderResult(int result) { 765 int HttpStreamParser::HandleReadHeaderResult(int result) {
766 DCHECK_EQ(0, read_buf_unused_offset_); 766 DCHECK_EQ(0, read_buf_unused_offset_);
767 767
768 if (result == 0) 768 if (result == 0)
769 result = ERR_CONNECTION_CLOSED; 769 result = ERR_CONNECTION_CLOSED;
770 770
771 if (result == ERR_CONNECTION_CLOSED) { 771 if (result < 0 && result != ERR_CONNECTION_CLOSED) {
772 if (read_buf_->offset() == 0) { 772 io_state_ = STATE_DONE;
773 io_state_ = STATE_DONE;
774 // If the connection has not been reused, it may have been a 0-length
775 // HTTP/0.9 responses, but it was most likely an error, so just return
776 // ERR_EMPTY_RESPONSE instead. If the connection was reused, just pass
777 // on the original connection close error, as rather than being an
778 // empty HTTP/0.9 response it's much more likely the server closed the
779 // socket before it received the request.
780 if (!connection_->is_reused())
781 return ERR_EMPTY_RESPONSE;
782 return result;
783 }
784
785 // The connection closed before the end of the headers. For HTTPS,
786 // accepting the partial headers headers is a potential security
787 // vulnerability. For HTTP, it just doesn't seem like a good idea.
788 if (response_header_start_offset_ >= 0) {
789 io_state_ = STATE_DONE;
790 return ERR_RESPONSE_HEADERS_TRUNCATED;
791 }
792
793 // The response is apparently using HTTP/0.9. Treat the entire response as
794 // the body.
795 int rv = ParseResponseHeaders(0);
796 if (rv < 0)
797 return rv;
798 return result; 773 return result;
799 } 774 }
800 775 // If we've used the connection before, then we know it is not a HTTP/0.9
801 if (result < 0) { 776 // response and return ERR_CONNECTION_CLOSED.
777 if (result == ERR_CONNECTION_CLOSED && read_buf_->offset() == 0 &&
778 connection_->is_reused()) {
802 io_state_ = STATE_DONE; 779 io_state_ = STATE_DONE;
803 return result; 780 return result;
804 } 781 }
805 782
806 // Record our best estimate of the 'response time' as the time when we read 783 // Record our best estimate of the 'response time' as the time when we read
807 // the first bytes of the response headers. 784 // the first bytes of the response headers.
808 if (read_buf_->offset() == 0) 785 if (read_buf_->offset() == 0 && result != ERR_CONNECTION_CLOSED)
809 response_->response_time = base::Time::Now(); 786 response_->response_time = base::Time::Now();
810 787
788 if (result == ERR_CONNECTION_CLOSED) {
789 // The connection closed before we detected the end of the headers.
790 if (read_buf_->offset() == 0) {
791 // The connection was closed before any data was sent. Likely an error
792 // rather than empty HTTP/0.9 response.
793 io_state_ = STATE_DONE;
794 return ERR_EMPTY_RESPONSE;
795 } else if (request_->url.SchemeIsCryptographic()) {
796 // The connection was closed in the middle of the headers. For HTTPS we
797 // don't parse partial headers. Return a different error code so that we
798 // know that we shouldn't attempt to retry the request.
799 io_state_ = STATE_DONE;
800 return ERR_RESPONSE_HEADERS_TRUNCATED;
801 }
802 // Parse things as well as we can and let the caller decide what to do.
803 int end_offset;
804 if (response_header_start_offset_ >= 0) {
805 // The response looks to be a truncated set of HTTP headers.
806 io_state_ = STATE_READ_BODY_COMPLETE;
807 end_offset = read_buf_->offset();
808 RecordHeaderParserEvent(HEADER_ALLOWED_TRUNCATED_HEADERS);
809 } else {
810 // The response is apparently using HTTP/0.9. Treat the entire response
811 // the body.
812 end_offset = 0;
813 }
814 int rv = ParseResponseHeaders(end_offset);
815 if (rv < 0)
816 return rv;
817 return result;
818 }
819
811 read_buf_->set_offset(read_buf_->offset() + result); 820 read_buf_->set_offset(read_buf_->offset() + result);
812 DCHECK_LE(read_buf_->offset(), read_buf_->capacity()); 821 DCHECK_LE(read_buf_->offset(), read_buf_->capacity());
813 DCHECK_GE(result, 0); 822 DCHECK_GE(result, 0);
814 823
815 int end_of_header_offset = FindAndParseResponseHeaders(); 824 int end_of_header_offset = FindAndParseResponseHeaders();
816 825
817 // Note: -1 is special, it indicates we haven't found the end of headers. 826 // Note: -1 is special, it indicates we haven't found the end of headers.
818 // Anything less than -1 is a net::Error, so we bail out. 827 // Anything less than -1 is a net::Error, so we bail out.
819 if (end_of_header_offset < -1) 828 if (end_of_header_offset < -1)
820 return end_of_header_offset; 829 return end_of_header_offset;
(...skipping 267 matching lines...) Expand 10 before | Expand all | Expand 10 after
1088 request_body->IsInMemory() && 1097 request_body->IsInMemory() &&
1089 request_body->size() > 0) { 1098 request_body->size() > 0) {
1090 uint64 merged_size = request_headers.size() + request_body->size(); 1099 uint64 merged_size = request_headers.size() + request_body->size();
1091 if (merged_size <= kMaxMergedHeaderAndBodySize) 1100 if (merged_size <= kMaxMergedHeaderAndBodySize)
1092 return true; 1101 return true;
1093 } 1102 }
1094 return false; 1103 return false;
1095 } 1104 }
1096 1105
1097 } // namespace net 1106 } // namespace net
OLDNEW
« no previous file with comments | « net/http/http_network_transaction_unittest.cc ('k') | net/http/http_stream_parser_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698