Removed redundant Http::StatusLine::protocol (#794)

Also polished Http::StatusLine initialization, but that part needs a
lot more work to get rid of Http::StatusLine::init().

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index ba1737778e8c7fc892fbfd7d87132ffb8a657d59..78e28d0bc86e9ad51a65c190c3ea91d5af86c39f 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -1463,7 +1463,7 @@ clientReplyContext::buildReplyHeader()
     hdr->putStr(Http::HdrType::CACHE_STATUS, cacheStatus.c_str());
 
     const bool maySendChunkedReply = !request->multipartRangeRequest() &&
-                                     reply->sline.protocol == AnyP::PROTO_HTTP && // response is HTTP
+                                     reply->sline.version.protocol == AnyP::PROTO_HTTP && // response is HTTP
                                      (request->http_ver >= Http::ProtocolVersion(1,1));
 
     /* Check whether we should send keep-alive */
@@ -1547,7 +1547,7 @@ clientReplyContext::cloneReply()
 
     http->al->reply = reply;
 
-    if (reply->sline.protocol == AnyP::PROTO_HTTP) {
+    if (reply->sline.version.protocol == AnyP::PROTO_HTTP) {
         /* RFC 2616 requires us to advertise our version (but only on real HTTP traffic) */
         reply->sline.version = Http::ProtocolVersion();
     }

diff --git a/src/http.cc b/src/http.cc
index ac89872370303526562cc51d6bced79c7a06ff72..4735d58d56a92f7773e5c7039134c9b25fe7724b 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -730,18 +730,11 @@ HttpStateData::processReplyHeader()
     // XXX: RFC 7230 indicates we MAY ignore the reason phrase,
     //      and use an empty string on unknown status.
     //      We do that now to avoid performance regression from using SBuf::c_str()
-    newrep->sline.set(Http::ProtocolVersion(1,1), hp->messageStatus() /* , hp->reasonPhrase() */);
-    newrep->sline.protocol = newrep->sline.version.protocol = hp->messageProtocol().protocol;
-    newrep->sline.version.major = hp->messageProtocol().major;
-    newrep->sline.version.minor = hp->messageProtocol().minor;
+    newrep->sline.set(hp->messageProtocol(), hp->messageStatus() /* , hp->reasonPhrase() */);
 
     // parse headers
     if (!newrep->parseHeader(*hp)) {
-        // XXX: when Http::ProtocolVersion is a function, remove this hack. just set with messageProtocol()
-        newrep->sline.set(Http::ProtocolVersion(), Http::scInvalidHeader);
-        newrep->sline.version.protocol = hp->messageProtocol().protocol;
-        newrep->sline.version.major = hp->messageProtocol().major;
-        newrep->sline.version.minor = hp->messageProtocol().minor;
+        newrep->sline.set(hp->messageProtocol(), Http::scInvalidHeader);
         debugs(11, 2, "error parsing response headers mime block");
     }
 
@@ -752,14 +745,14 @@ HttpStateData::processReplyHeader()
 
     newrep->removeStaleWarnings();
 
-    if (newrep->sline.protocol == AnyP::PROTO_HTTP && Http::Is1xx(newrep->sline.status())) {
+    if (newrep->sline.version.protocol == AnyP::PROTO_HTTP && Http::Is1xx(newrep->sline.status())) {
         handle1xx(newrep);
         ctx_exit(ctx);
         return;
     }
 
     flags.chunked = false;
-    if (newrep->sline.protocol == AnyP::PROTO_HTTP && newrep->header.chunked()) {
+    if (newrep->sline.version.protocol == AnyP::PROTO_HTTP && newrep->header.chunked()) {
         flags.chunked = true;
         httpChunkDecoder = new Http1::TeChunkedParser;
     }

diff --git a/src/http/StatusLine.cc b/src/http/StatusLine.cc
index 10ed47deaddb5f7c11e932ab3a9378c38a2b771c..a284a4cbfeabf936b8cd14860ec0adee6e3c00bc 100644 (file)
--- a/src/http/StatusLine.cc
+++ b/src/http/StatusLine.cc
@@ -34,7 +34,6 @@ Http::StatusLine::clean()
 void
 Http::StatusLine::set(const AnyP::ProtocolVersion &newVersion, const Http::StatusCode newStatus, const char *newReason)
 {
-    protocol = AnyP::PROTO_HTTP;
     version = newVersion;
     status_ = newStatus;
     /* Note: no xstrdup for 'reason', assumes constant 'reasons' */
@@ -69,7 +68,7 @@ Http::StatusLine::packInto(Packable * p) const
     static const char *IcyStatusLineFormat = "ICY %3d %s\r\n";
 
     /* handle ICY protocol status line specially. Pass on the bad format. */
-    if (protocol == AnyP::PROTO_ICY) {
+    if (version.protocol == AnyP::PROTO_ICY) {
         debugs(57, 9, "packing sline " << this << " using " << p << ":");
         debugs(57, 9, "FORMAT=" << IcyStatusLineFormat );
         debugs(57, 9, "ICY " << packedStatus << " " << packedReason);
@@ -93,7 +92,7 @@ Http::StatusLine::parse(const String &protoPrefix, const char *start, const char
 
     if (protoPrefix.cmp("ICY", 3) == 0) {
         debugs(57, 3, "Invalid HTTP identifier. Detected ICY protocol instead.");
-        protocol = AnyP::PROTO_ICY;
+        version = AnyP::ProtocolVersion(AnyP::PROTO_ICY, 1, 0);
         start += protoPrefix.size();
     } else if (protoPrefix.caseCmp(start, protoPrefix.size()) == 0) {
 

diff --git a/src/http/StatusLine.h b/src/http/StatusLine.h
index 9df3bdb968f85d2ee41872a1f665cbe9dad40019..0adeeed4b683bb59193349adc1f7dd885c20f542 100644 (file)
--- a/src/http/StatusLine.h
+++ b/src/http/StatusLine.h
@@ -20,7 +20,7 @@ namespace Http
 {
 
 /**
- * Holds the values parsed from an HTTP reply status line.
+ * Holds the values parsed from an HTTP-like reply status line.
  *
  * For example: HTTP/1.1 200 OK
  */
@@ -59,22 +59,14 @@ public:
 public:
     /* public, read only */
 
-    /**
-     * By rights protocol name should be a constant "HTTP", with no need for this field to exist.
-     * However there are protocols which violate HTTP by sending their own custom formats
-     * back with other protocol names (ICY streaming format being the current major problem).
-     */
-    // XXX: protocol is part of AnyP::ProtocolVersion. We should be able to use version.protocol instead now.
-    AnyP::ProtocolType protocol;
-
     AnyP::ProtocolVersion version;     ///< breakdown of protocol version label: (HTTP/ICY) and (0.9/1.0/1.1)
 
 private:
     /// status code. ie 100 ... 200 ... 404 ... 599
-    Http::StatusCode status_;
+    Http::StatusCode status_ = scNone;
 
     /// points to a _constant_ string (default or supplied), never free()d
-    const char *reason_;
+    const char *reason_ = nullptr;
 };
 
 } // namespace Http