From: Mike Stepanek (mstepane) <mstepane@cisco.com>
Date: Mon, 28 Sep 2020 19:58:16 +0000 (+0000)
Subject: Merge pull request #2500 in SNORT/snort3 from ~THOPETER/snort3:h2i6 to master
X-Git-Tag: 3.0.3-2~23
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4ee14485cda34ceda2e85dd5a52407967128b449;p=thirdparty%2Fsnort3.git

Merge pull request #2500 in SNORT/snort3 from ~THOPETER/snort3:h2i6 to master

Squashed commit of the following:

commit e7e8f2c22e796db2fe55cc202f02a55f2c76bf80
Author: Tom Peters <thopeter@cisco.com>
Date:   Tue Sep 15 19:49:55 2020 -0400

    http2_inspect: stream state tracking
---

diff --git a/src/service_inspectors/http2_inspect/dev_notes.txt b/src/service_inspectors/http2_inspect/dev_notes.txt
index 23fb9cdfb..83b4daf31 100644
--- a/src/service_inspectors/http2_inspect/dev_notes.txt
+++ b/src/service_inspectors/http2_inspect/dev_notes.txt
@@ -22,7 +22,7 @@ frame_data buffer. The frame_data buffer is passed to the function decode_header
 main loop driving HPACK decoding. Each decoded header line is progressively written to a second
 decoded_headers buffer that will ultimately be sent to NHI.
 
-The main loop in decode_headers() finds the cut point for a single header line. The line is is
+The main loop in decode_headers() finds the cut point for a single header line. The line is
 passed to decode_header_line(), which parses the line and calls the appropriate decoding function
 based on the header representation type. If the type is indexed, the full header line is looked up
 in the table and copied to the decoded header buffer. The index may belong to either the static or
@@ -33,6 +33,16 @@ literal. The resulting header line is then added to the dynamic table. The third
 is literal not to be indexed, which is the same as literal to be indexed, except the header line is
 not added to the dynamic table.
 
+H2I has two levels of failure for flow processing. Fatal errors include failures in frame splitting
+and errors in header decoding that compromise the HPACK dictionary. A fatal error will trigger an
+immediate EVENT_MISFORMATTED_HTTP2 and will cause scan() to return ABORT the next time it is called
+in the same direction.
+
+Stream errors affect HTTP/1 processing within a stream. They include errors in frame sequence
+within a stream, errors that cause the HI stream splitter to abort, and errors that cause HI to be
+unable to process a frame. The individual stream will transition to STREAM_ERROR state and HI
+processing of that stream in that direction will end.
+
 H2I supports the NHI test tool. See ../http_inspect/dev_notes.txt for usage instructions.
 
 Memory requirements: Http2FlowData represents all H2I information in a flow. It does not account 
diff --git a/src/service_inspectors/http2_inspect/http2_data_frame.cc b/src/service_inspectors/http2_inspect/http2_data_frame.cc
index ba57bfab4..aa1e40781 100644
--- a/src/service_inspectors/http2_inspect/http2_data_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_data_frame.cc
@@ -35,18 +35,27 @@ using namespace HttpCommon;
 using namespace snort;
 using namespace Http2Enums;
 
-Http2DataFrame::Http2DataFrame(const uint8_t* header_buffer, const int32_t header_len,
-    const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* session_data_,
+Http2DataFrame::Http2DataFrame(const uint8_t* header_buffer, const uint32_t header_len,
+    const uint8_t* data_buffer_, const uint32_t data_length_, Http2FlowData* session_data_,
     HttpCommon::SourceId source_id_, Http2Stream* stream_) :
     Http2Frame(header_buffer, header_len, nullptr, 0, session_data_, source_id_, stream_),
-    data_length(data_len)
+    data_length(data_length_),
+    data_buffer(data_buffer_)
+{}
+
+bool Http2DataFrame::valid_sequence(Http2Enums::StreamState state)
+{
+    return (state == Http2Enums::STREAM_EXPECT_BODY) || (state == Http2Enums::STREAM_BODY);
+}
+
+void Http2DataFrame::analyze_http1()
 {
-    if ((data_len != 0) || !session_data->flushing_data[source_id])
+    if ((data_length != 0) || !session_data->flushing_data[source_id])
     {
         Http2DummyPacket dummy_pkt;
         dummy_pkt.flow = session_data->flow;
         dummy_pkt.packet_flags = (source_id == SRC_CLIENT) ? PKT_FROM_CLIENT : PKT_FROM_SERVER;
-        dummy_pkt.dsize = data_len;
+        dummy_pkt.dsize = data_length;
         dummy_pkt.data = data_buffer;
         dummy_pkt.xtradata_mask = 0;
         session_data->hi->eval(&dummy_pkt);
@@ -56,8 +65,8 @@ Http2DataFrame::Http2DataFrame(const uint8_t* header_buffer, const int32_t heade
     else
     {
         detection_required = true;
-        HttpFlowData* const http_flow = (HttpFlowData*)session_data_->get_hi_flow_data();
-        http_flow->reset_partial_flush(source_id_);
+        HttpFlowData* const http_flow = (HttpFlowData*)session_data->get_hi_flow_data();
+        http_flow->reset_partial_flush(source_id);
     }
 }
 
diff --git a/src/service_inspectors/http2_inspect/http2_data_frame.h b/src/service_inspectors/http2_inspect/http2_data_frame.h
index b16db7c75..86c534328 100644
--- a/src/service_inspectors/http2_inspect/http2_data_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_data_frame.h
@@ -29,24 +29,27 @@ class Http2DataFrame : public Http2Frame
 {
 public:
     ~Http2DataFrame() override {}
+    bool valid_sequence(Http2Enums::StreamState state) override;
+    void analyze_http1() override;
     void clear() override;
 
     uint32_t get_xtradata_mask() override { return xtradata_mask; }
     bool is_detection_required() const override { return detection_required; }
     void update_stream_state() override;
 
-    friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const int32_t, const uint8_t*,
-        const int32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+    friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
 
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
 #endif
 
 private:
-    Http2DataFrame(const uint8_t* header_buffer, const int32_t header_len,
-        const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* ssn_data,
+    Http2DataFrame(const uint8_t* header_buffer, const uint32_t header_len,
+        const uint8_t* data_buffer_, const uint32_t data_length_, Http2FlowData* ssn_data,
         HttpCommon::SourceId src_id, Http2Stream* stream);
-    uint32_t data_length = 0;
+    const uint32_t data_length;
+    const uint8_t* const data_buffer;
     uint32_t xtradata_mask = 0;
     bool detection_required = false;
 };
diff --git a/src/service_inspectors/http2_inspect/http2_enum.h b/src/service_inspectors/http2_inspect/http2_enum.h
index 286077501..a57c551b4 100644
--- a/src/service_inspectors/http2_inspect/http2_enum.h
+++ b/src/service_inspectors/http2_inspect/http2_enum.h
@@ -33,11 +33,12 @@ static const uint32_t HTTP2_GID = 121;
 
 // Frame type codes (fourth octet of frame header)
 enum FrameType : uint8_t { FT_DATA=0, FT_HEADERS=1, FT_PRIORITY=2, FT_RST_STREAM=3, FT_SETTINGS=4,
-    FT_PUSH_PROMISE=5, FT_PING=6, FT_GOAWAY=7, FT_WINDOW_UPDATE=8, FT_CONTINUATION=9, FT__ABORT=254,
+    FT_PUSH_PROMISE=5, FT_PING=6, FT_GOAWAY=7, FT_WINDOW_UPDATE=8, FT_CONTINUATION=9,
     FT__NONE=255 };
 
-enum StreamState { STREAM_EXPECT_HEADERS, STREAM_EXPECT_BODY, STREAM_BODY, STREAM_IMPLIED_COMPLETE,
-    STREAM_COMPLETE, STREAM_ERROR };
+// Ordered from initial state to terminal state. Do not rearrange without careful consideration.
+enum StreamState { STREAM_EXPECT_HEADERS, STREAM_EXPECT_BODY, STREAM_BODY, STREAM_COMPLETE,
+    STREAM_ERROR };
 
 // Message buffers available to clients
 // This enum must remain synchronized with Http2Api::classic_buffer_names[]
@@ -71,6 +72,7 @@ enum EventSid
     EVENT_PSEUDO_HEADER_AFTER_REGULAR_HEADER = 17,
     EVENT_PSEUDO_HEADER_IN_TRAILERS = 18,
     EVENT_INVALID_PSEUDO_HEADER = 19,
+    EVENT_TRAILERS_NOT_END = 20,
     EVENT__MAX_VALUE
 };
 
@@ -106,8 +108,9 @@ enum Infraction
     INF_INVALID_STARTLINE = 25,
     INF_INVALID_HEADER = 26,
     INF_PADDING_LEN = 27,
-    INF_TRAILERS_AFTER_END_STREAM = 28,
+    INF_UNUSED_2 = 28,
     INF_PSEUDO_HEADER_IN_TRAILERS = 29,
+    INF_TRAILERS_NOT_END = 30,
     INF__MAX_VALUE
 };
 
diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h
index 57b43fd13..98fc44835 100644
--- a/src/service_inspectors/http2_inspect/http2_flow_data.h
+++ b/src/service_inspectors/http2_inspect/http2_flow_data.h
@@ -160,6 +160,7 @@ protected:
 
     // Used by scan, reassemble and eval to communicate
     uint8_t frame_type[2] = { Http2Enums::FT__NONE, Http2Enums::FT__NONE };
+    bool abort_flow[2] = { false, false };
 
     // Internal to reassemble()
     uint32_t frame_header_offset[2] = { 0, 0 };
diff --git a/src/service_inspectors/http2_inspect/http2_frame.cc b/src/service_inspectors/http2_inspect/http2_frame.cc
index 4d01db283..8da611638 100644
--- a/src/service_inspectors/http2_inspect/http2_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_frame.cc
@@ -36,19 +36,19 @@ using namespace HttpCommon;
 using namespace Http2Enums;
 using namespace snort;
 
-Http2Frame::Http2Frame(const uint8_t* header_buffer, const int32_t header_len,
-    const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* session_data,
-    SourceId source_id, Http2Stream* stream_) :  session_data(session_data), source_id(source_id),
-    stream(stream_)
+Http2Frame::Http2Frame(const uint8_t* header_buffer, const uint32_t header_len,
+    const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* session_data,
+    SourceId source_id, Http2Stream* stream_) :
+    session_data(session_data), source_id(source_id), stream(stream_)
 {
-    if (header_len > 0)
-        header.set(header_len, header_buffer, true);
+    header.set(header_len, header_buffer, true);
+    // FIXIT-E want to refactor so that zero-length frames are not a special case
     if (data_len > 0)
         data.set(data_len, data_buffer, true);
 }
 
-Http2Frame* Http2Frame::new_frame(const uint8_t* header, const int32_t header_len,
-    const uint8_t* data, const int32_t data_len, Http2FlowData* session_data, SourceId source_id,
+Http2Frame* Http2Frame::new_frame(const uint8_t* header, const uint32_t header_len,
+    const uint8_t* data, const uint32_t data_len, Http2FlowData* session_data, SourceId source_id,
     Http2Stream* stream)
 {
     // FIXIT-E call the appropriate frame subclass constructor based on the type
diff --git a/src/service_inspectors/http2_inspect/http2_frame.h b/src/service_inspectors/http2_inspect/http2_frame.h
index 73380eeb8..6a2789734 100644
--- a/src/service_inspectors/http2_inspect/http2_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_frame.h
@@ -40,8 +40,8 @@ class Http2Frame
 {
 public:
     virtual ~Http2Frame() { }
-    static Http2Frame* new_frame(const uint8_t* header_buffer, const int32_t header_len,
-        const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* session_data,
+    static Http2Frame* new_frame(const uint8_t* header_buffer, const uint32_t header_len,
+        const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* session_data,
         HttpCommon::SourceId source_id, Http2Stream* stream);
     virtual bool valid_sequence(Http2Enums::StreamState state)
         { return state != Http2Enums::STREAM_ERROR; }
@@ -57,8 +57,8 @@ public:
 #endif
 
 protected:
-    Http2Frame(const uint8_t* header_buffer, const int32_t header_len,
-        const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* session_data,
+    Http2Frame(const uint8_t* header_buffer, const uint32_t header_len,
+        const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* session_data,
         HttpCommon::SourceId source_id, Http2Stream* stream);
     uint8_t get_flags();
     uint32_t get_stream_id();
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.cc b/src/service_inspectors/http2_inspect/http2_headers_frame.cc
index b7dddc5a5..02ab6eaae 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame.cc
@@ -40,13 +40,13 @@ using namespace snort;
 using namespace HttpCommon;
 using namespace Http2Enums;
 
-Http2HeadersFrame::Http2HeadersFrame(const uint8_t* header_buffer, const int32_t header_len,
-    const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* session_data_,
+Http2HeadersFrame::Http2HeadersFrame(const uint8_t* header_buffer, const uint32_t header_len,
+    const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* session_data_,
     HttpCommon::SourceId source_id_, Http2Stream* stream_) :
     Http2Frame(header_buffer, header_len, data_buffer, data_len, session_data_, source_id_, stream_)
 {
-    // FIXIT-E check frame validity before creating frame
-    if (!check_frame_validity() or data.length() <= 0)
+    // FIXIT-E zero length should not be a special case
+    if (data.length() <= 0)
     {
         process_frame = false;
         return;
@@ -64,13 +64,12 @@ Http2HeadersFrame::Http2HeadersFrame(const uint8_t* header_buffer, const int32_t
 
 Http2HeadersFrame::~Http2HeadersFrame()
 {
-    delete http1_header;
     delete[] decoded_headers;
 }
 
 void Http2HeadersFrame::clear()
 {
-    if (error_during_decode || hi_abort)
+    if (session_data->abort_flow[source_id] || stream->get_state(source_id) == STREAM_ERROR)
         return;
     Packet dummy_pkt(false);
     dummy_pkt.flow = session_data->flow;
@@ -79,7 +78,7 @@ void Http2HeadersFrame::clear()
 
 void Http2HeadersFrame::process_decoded_headers(HttpFlowData* http_flow)
 {
-    if (error_during_decode)
+    if (session_data->abort_flow[source_id])
         return;
 
     http1_header = hpack_decoder->get_decoded_headers(decoded_headers);
@@ -92,21 +91,21 @@ void Http2HeadersFrame::process_decoded_headers(HttpFlowData* http_flow)
         dummy_pkt.flow = session_data->flow;
         const uint32_t unused = 0;
         const StreamSplitter::Status header_scan_result =
-            session_data->hi_ss[source_id]->scan(&dummy_pkt, http1_header->start(),
-            http1_header->length(), unused, &flush_offset);
+            session_data->hi_ss[source_id]->scan(&dummy_pkt, http1_header.start(),
+            http1_header.length(), unused, &flush_offset);
         assert(header_scan_result == StreamSplitter::FLUSH);
         UNUSED(header_scan_result);
-        assert((int64_t)flush_offset == http1_header->length());
+        assert((int64_t)flush_offset == http1_header.length());
     }
 
     // http_inspect reassemble() of headers
     {
         unsigned copied;
         stream_buf = session_data->hi_ss[source_id]->reassemble(session_data->flow,
-            http1_header->length(), 0, http1_header->start(), http1_header->length(), PKT_PDU_TAIL,
+            http1_header.length(), 0, http1_header.start(), http1_header.length(), PKT_PDU_TAIL,
             copied);
         assert(stream_buf.data != nullptr);
-        assert(copied == (unsigned)http1_header->length());
+        assert(copied == (unsigned)http1_header.length());
     }
 
     // http_inspect eval() of headers
@@ -118,15 +117,14 @@ void Http2HeadersFrame::process_decoded_headers(HttpFlowData* http_flow)
         dummy_pkt.data = stream_buf.data;
         dummy_pkt.xtradata_mask = 0;
         session_data->hi->eval(&dummy_pkt);
-        //Following if condition won't get exercised until finish() is
-        //implemented for H2I. Without finish() H2I will only flush
-        //complete header blocks. Below ABORT is only possible if
-        //tcp connection closes unexpectedly in middle of a header.
+        // Following if condition won't get exercised until finish() (during Headers) is
+        // implemented for H2I. Without finish() H2I will only flush complete header blocks. Below
+        // ABORT is only possible if tcp connection closes unexpectedly in middle of a header.
         if (http_flow->get_type_expected(source_id) == HttpEnums::SEC_ABORT)
         {
             *session_data->infractions[source_id] += INF_INVALID_HEADER;
             session_data->events[source_id]->create_event(EVENT_INVALID_HEADER);
-            hi_abort = true;
+            stream->set_state(source_id, STREAM_ERROR);
             return;
         }
         detection_required = dummy_pkt.is_detection_required();
@@ -140,67 +138,16 @@ const Field& Http2HeadersFrame::get_buf(unsigned id)
     {
     // FIXIT-M need to add a buffer for the decoded start line
     case HTTP2_BUFFER_DECODED_HEADER:
-        return *http1_header;
+        return http1_header;
     default:
         return Http2Frame::get_buf(id);
     }
 }
 
-// Return: continue processing frame
-bool Http2HeadersFrame::check_frame_validity()
-{
-    if (stream->get_state(source_id) == STREAM_COMPLETE)
-    {
-        *session_data->infractions[source_id] += INF_TRAILERS_AFTER_END_STREAM;
-        session_data->events[source_id]->create_event(EVENT_FRAME_SEQUENCE);
-        return false;
-    }
-    else if ((stream->get_state(source_id) != STREAM_EXPECT_HEADERS) and
-        !(get_flags() & END_STREAM))
-    {
-        // Trailers without END_STREAM flag set. Alert but continue to process.
-        *session_data->infractions[source_id] += INF_FRAME_SEQUENCE;
-        session_data->events[source_id]->create_event(EVENT_FRAME_SEQUENCE);
-    }
-    return true;
-}
-
-void Http2HeadersFrame::update_stream_state()
-{
-    switch (stream->get_state(source_id))
-    {
-        case STREAM_EXPECT_HEADERS:
-            if (get_flags() & END_STREAM)
-                stream->set_state(source_id, STREAM_COMPLETE);
-            else
-                stream->set_state(source_id, STREAM_EXPECT_BODY);
-            break;
-        case STREAM_EXPECT_BODY:
-            // fallthrough
-        case STREAM_BODY:
-            // Any trailing headers frame will be processed as trailers regardless of whether the
-            // END_STREAM flag is set
-            if (stream->get_state(source_id) == STREAM_BODY)
-                session_data->concurrent_files -= 1;
-            stream->set_state(source_id, STREAM_COMPLETE);
-            break;
-        case STREAM_COMPLETE:
-            // FIXIT-E frame validity should be checked before creating frame so we should not get
-            // here
-            break;
-        default:
-            // FIXIT-E build this out
-            break;
-    }
-}
-
 #ifdef REG_TEST
 void Http2HeadersFrame::print_frame(FILE* output)
 {
-    if (error_during_decode)
-        fprintf(output, "Error decoding headers.\n");
-    if (http1_header)
-        http1_header->print(output, "Decoded header");
+    http1_header.print(output, "Decoded header");
     Http2Frame::print_frame(output);
 }
 #endif
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.h b/src/service_inspectors/http2_inspect/http2_headers_frame.h
index 4e861b58c..7f7c4904c 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame.h
@@ -38,24 +38,19 @@ public:
     const Field& get_buf(unsigned id) override;
     uint32_t get_xtradata_mask() override { return xtradata_mask; }
     bool is_detection_required() const override { return detection_required; }
-    void update_stream_state() override;
 
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
 #endif
 
 protected:
-    Http2HeadersFrame(const uint8_t* header_buffer, const int32_t header_len,
-        const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* ssn_data,
+    Http2HeadersFrame(const uint8_t* header_buffer, const uint32_t header_len,
+        const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
         HttpCommon::SourceId src_id, Http2Stream* stream);
-    bool check_frame_validity();
     void process_decoded_headers(HttpFlowData* http_flow);
 
     uint8_t* decoded_headers = nullptr; // working buffer to store decoded headers
-    uint32_t decoded_headers_size = 0;
-    const Field* http1_header = nullptr; // finalized headers to be passed to NHI
-    bool error_during_decode = false;
-    bool hi_abort = false;
+    Field http1_header;                 // finalized headers to be passed to NHI
     uint32_t xtradata_mask = 0;
     bool detection_required = false;
     bool process_frame = true;
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc b/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
index 573466a53..b7bc20bf0 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
@@ -40,9 +40,9 @@ using namespace snort;
 using namespace HttpCommon;
 using namespace Http2Enums;
 
-Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer, const int32_t header_len,
-    const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* session_data_,
-    HttpCommon::SourceId source_id_, Http2Stream* stream_) :
+Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer,
+    const uint32_t header_len, const uint8_t* data_buffer, const uint32_t data_len,
+    Http2FlowData* session_data_, HttpCommon::SourceId source_id_, Http2Stream* stream_) :
     Http2HeadersFrame(header_buffer, header_len, data_buffer, data_len, session_data_, source_id_,
         stream_)
 {
@@ -56,27 +56,33 @@ Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer, c
     if (!hpack_decoder->decode_headers((data.start() + hpack_headers_offset), data.length() -
         hpack_headers_offset, decoded_headers, start_line_generator, false))
     {
-        session_data->frame_type[source_id] = FT__ABORT;
-        error_during_decode = true;
+        session_data->abort_flow[source_id] = true;
+        session_data->events[source_id]->create_event(EVENT_MISFORMATTED_HTTP2);
+        return;
     }
 
     // process start line
     if (!start_line_generator->generate_start_line(start_line))
     {
-        // FIXIT-E set stream state to error
-        return;
+        // FIXIT-E should only be a stream error
+        session_data->abort_flow[source_id] = true;
+        session_data->events[source_id]->create_event(EVENT_MISFORMATTED_HTTP2);
     }
 }
 
 Http2HeadersFrameHeader::~Http2HeadersFrameHeader()
 {
-    delete start_line;
     delete start_line_generator;
 }
 
+bool Http2HeadersFrameHeader::valid_sequence(Http2Enums::StreamState state)
+{
+    return (state == Http2Enums::STREAM_EXPECT_HEADERS);
+}
+
 void Http2HeadersFrameHeader::analyze_http1()
 {
-    if (!process_frame || start_line == nullptr)
+    if (!process_frame)
         return;
 
     // http_inspect scan() of start line
@@ -86,11 +92,11 @@ void Http2HeadersFrameHeader::analyze_http1()
         dummy_pkt.flow = session_data->flow;
         const uint32_t unused = 0;
         const StreamSplitter::Status start_scan_result =
-            session_data->hi_ss[source_id]->scan(&dummy_pkt, start_line->start(),
-                start_line->length(), unused, &flush_offset);
+            session_data->hi_ss[source_id]->scan(&dummy_pkt, start_line.start(),
+                start_line.length(), unused, &flush_offset);
         assert(start_scan_result == StreamSplitter::FLUSH);
         UNUSED(start_scan_result);
-        assert((int64_t)flush_offset == start_line->length());
+        assert((int64_t)flush_offset == start_line.length());
     }
 
     StreamBuffer stream_buf;
@@ -99,10 +105,10 @@ void Http2HeadersFrameHeader::analyze_http1()
     {
         unsigned copied;
         stream_buf = session_data->hi_ss[source_id]->reassemble(session_data->flow,
-            start_line->length(), 0, start_line->start(), start_line->length(), PKT_PDU_TAIL,
+            start_line.length(), 0, start_line.start(), start_line.length(), PKT_PDU_TAIL,
             copied);
         assert(stream_buf.data != nullptr);
-        assert(copied == (unsigned)start_line->length());
+        assert(copied == (unsigned)start_line.length());
     }
 
     HttpFlowData* const http_flow =
@@ -119,7 +125,7 @@ void Http2HeadersFrameHeader::analyze_http1()
         {
             *session_data->infractions[source_id] += INF_INVALID_STARTLINE;
             session_data->events[source_id]->create_event(EVENT_INVALID_STARTLINE);
-            hi_abort = true;
+            stream->set_state(source_id, STREAM_ERROR);
             return;
         }
         session_data->hi->clear(&dummy_pkt);
@@ -128,14 +134,21 @@ void Http2HeadersFrameHeader::analyze_http1()
     process_decoded_headers(http_flow);
 }
 
+void Http2HeadersFrameHeader::update_stream_state()
+{
+    if (stream->get_state(source_id) == STREAM_ERROR)
+        return;
+    if (get_flags() & END_STREAM)
+        stream->set_state(source_id, STREAM_COMPLETE);
+    else
+        stream->set_state(source_id, STREAM_EXPECT_BODY);
+}
+
 #ifdef REG_TEST
 void Http2HeadersFrameHeader::print_frame(FILE* output)
 {
     fprintf(output, "Headers frame\n");
-    if (start_line)
-        start_line->print(output, "Decoded start-line");
-    else
-        fprintf(output, "Error generating start line.\n");
+    start_line.print(output, "Decoded start-line");
     Http2HeadersFrame::print_frame(output);
 }
 #endif
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_header.h b/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
index 28d4778f0..13b5e3ab0 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
@@ -30,21 +30,23 @@ class Http2HeadersFrameHeader : public Http2HeadersFrame
 public:
     ~Http2HeadersFrameHeader() override;
     
-    friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const int32_t, const uint8_t*,
-        const int32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+    friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
 
+    bool valid_sequence(Http2Enums::StreamState state) override;
     void analyze_http1() override;
+    void update_stream_state() override;
 
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
 #endif
 
 private:
-    Http2HeadersFrameHeader(const uint8_t* header_buffer, const int32_t header_len,
-        const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* ssn_data,
+    Http2HeadersFrameHeader(const uint8_t* header_buffer, const uint32_t header_len,
+        const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
         HttpCommon::SourceId src_id, Http2Stream* stream);
 
     Http2StartLine* start_line_generator = nullptr;
-    const Field* start_line = nullptr;
+    Field start_line;
 };
 #endif
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
index 622af5138..4739e65a3 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
@@ -39,24 +39,36 @@ using namespace snort;
 using namespace HttpCommon;
 using namespace Http2Enums;
 
-Http2HeadersFrameTrailer::Http2HeadersFrameTrailer(const uint8_t* header_buffer, const int32_t header_len,
-    const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* session_data_,
-    HttpCommon::SourceId source_id_, Http2Stream* stream_) :
+Http2HeadersFrameTrailer::Http2HeadersFrameTrailer(const uint8_t* header_buffer,
+    const uint32_t header_len, const uint8_t* data_buffer, const uint32_t data_len,
+    Http2FlowData* session_data_, HttpCommon::SourceId source_id_, Http2Stream* stream_) :
     Http2HeadersFrame(header_buffer, header_len, data_buffer, data_len, session_data_, source_id_,
         stream_)
 {
     if (!process_frame)
         return;
 
+    if (!(get_flags() & END_STREAM))
+    {
+        // Trailers without END_STREAM flag set.
+        *session_data->infractions[source_id] += INF_TRAILERS_NOT_END;
+        session_data->events[source_id]->create_event(EVENT_TRAILERS_NOT_END);
+    }
+
     // Decode trailers
     if (!hpack_decoder->decode_headers((data.start() + hpack_headers_offset), data.length() -
         hpack_headers_offset, decoded_headers, nullptr, true))
     {
-        session_data->frame_type[source_id] = FT__ABORT;
-        error_during_decode = true;
+        session_data->abort_flow[source_id] = true;
+        session_data->events[source_id]->create_event(EVENT_MISFORMATTED_HTTP2);
     }
 }
 
+bool Http2HeadersFrameTrailer::valid_sequence(Http2Enums::StreamState state)
+{
+    return (state == Http2Enums::STREAM_EXPECT_BODY) || (state == Http2Enums::STREAM_BODY);
+}
+
 void Http2HeadersFrameTrailer::analyze_http1()
 {
     if (!process_frame)
@@ -90,7 +102,7 @@ void Http2HeadersFrameTrailer::analyze_http1()
         assert (http_flow->get_type_expected(source_id) == HttpEnums::SEC_TRAILER);
         if (http_flow->get_type_expected(source_id) == HttpEnums::SEC_ABORT)
         {
-            hi_abort = true;
+            stream->set_state(source_id, STREAM_ERROR);
             return;
         }
         session_data->hi->clear(&dummy_pkt);
@@ -99,6 +111,21 @@ void Http2HeadersFrameTrailer::analyze_http1()
     process_decoded_headers(http_flow);
 }
 
+void Http2HeadersFrameTrailer::update_stream_state()
+{
+    switch (stream->get_state(source_id))
+    {
+        case STREAM_BODY:
+            session_data->concurrent_files -= 1;
+            // fallthrough
+        case STREAM_EXPECT_BODY:
+            stream->set_state(source_id, STREAM_COMPLETE);
+            break;
+        default:
+            break;
+    }
+}
+
 #ifdef REG_TEST
 void Http2HeadersFrameTrailer::print_frame(FILE* output)
 {
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
index 1c12ba6bb..a011ad311 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
@@ -26,18 +26,20 @@
 class Http2HeadersFrameTrailer : public Http2HeadersFrame
 {
 public:
-    friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const int32_t, const uint8_t*,
-        const int32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+    friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
 
+    bool valid_sequence(Http2Enums::StreamState state) override;
     void analyze_http1() override;
+    void update_stream_state() override;
 
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
 #endif
 
 private:
-    Http2HeadersFrameTrailer(const uint8_t* header_buffer, const int32_t header_len,
-        const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* ssn_data,
+    Http2HeadersFrameTrailer(const uint8_t* header_buffer, const uint32_t header_len,
+        const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
         HttpCommon::SourceId src_id, Http2Stream* stream);
 };
 #endif
diff --git a/src/service_inspectors/http2_inspect/http2_hpack.cc b/src/service_inspectors/http2_inspect/http2_hpack.cc
index a9698fc24..ab40feb8a 100644
--- a/src/service_inspectors/http2_inspect/http2_hpack.cc
+++ b/src/service_inspectors/http2_inspect/http2_hpack.cc
@@ -49,7 +49,6 @@ bool Http2HpackDecoder::write_decoded_headers(const uint8_t* in_buffer, const ui
     {
         length = decoded_header_length;
         *infractions += INF_DECODED_HEADER_BUFF_OUT_OF_SPACE;
-        events->create_event(EVENT_MISFORMATTED_HTTP2);
         ret = false;
     }
 
@@ -94,7 +93,6 @@ const HpackTableEntry* Http2HpackDecoder::get_hpack_table_entry(
     if (!entry)
     {
         *infractions += INF_HPACK_INDEX_OUT_OF_BOUNDS;
-        events->create_event(EVENT_MISFORMATTED_HTTP2);
     }
     return entry;
 }
@@ -200,10 +198,10 @@ bool Http2HpackDecoder::decode_indexed_header(const uint8_t* encoded_header_buff
     name.set(entry->name);
     value.set(entry->value);
 
+    // FIXIT-E how bad is this?
     if (value.length() <= 0)
     {
         *infractions += INF_LOOKUP_EMPTY_VALUE;
-        events->create_event(EVENT_MISFORMATTED_HTTP2);
     }
 
     if (!write_header_part(name, (const uint8_t*)": ", 2, decoded_header_buffer,
@@ -249,27 +247,25 @@ bool Http2HpackDecoder::handle_dynamic_size_update(const uint8_t* encoded_header
     }
     bytes_consumed += encoded_bytes_consumed;
 
+    // Table size update shenanigans are dangerous because we cannot be sure how the target will
+    // interpret them.
     if (!table_size_update_allowed)
     {
         *infractions += INF_TABLE_SIZE_UPDATE_WITHIN_HEADER;
-        events->create_event(EVENT_MISFORMATTED_HTTP2);
-        return true;
+        return false;
     }
-    if (num_table_size_updates >= 2)
+    if (++num_table_size_updates > 2)
     {
         *infractions += INF_TOO_MANY_TABLE_SIZE_UPDATES;
-        events->create_event(EVENT_MISFORMATTED_HTTP2);
-        return true;
+        return false;
     }
 
     if (!decode_table.hpack_table_size_update(decoded_int))
     {
         *infractions += INF_INVALID_TABLE_SIZE_UPDATE;
-        events->create_event(EVENT_MISFORMATTED_HTTP2);
+        return false;
     }
 
-    num_table_size_updates++;
-
     return true;
 }
 
@@ -344,8 +340,8 @@ bool Http2HpackDecoder::decode_header_line(const uint8_t* encoded_header_buffer,
 
 // Entry point to decode an HPACK-encoded header block. This function returns true on successful
 // decode and false on an unrecoverable decode error. Note that alerts may still be generated for
-// recoverable errors while the function returns true. This function performs all decoding, but does
-// not output the start line or decoded headers - this function must be followed by calls to
+// recoverable errors while the function returns true. This function performs all decoding, but
+// does not output the start line or decoded headers - this function must be followed by calls to
 // generate_start_line() and get_decoded_headers() to generate and obtain these fields.
 bool Http2HpackDecoder::decode_headers(const uint8_t* encoded_headers,
     const uint32_t encoded_headers_length, uint8_t* decoded_headers,
@@ -357,11 +353,11 @@ bool Http2HpackDecoder::decode_headers(const uint8_t* encoded_headers,
     bool success = true;
     start_line = start_line_generator;
     decoded_headers_size = 0;
-    decode_error = false;
     is_trailers = trailers;
     pseudo_headers_allowed = !is_trailers;
 
-    // A maximum of two table size updates are allowed, and must be at the start of the header block
+    // A maximum of two table size updates are allowed, and must be at the start of the header
+    // block
     table_size_update_allowed = true;
     num_table_size_updates = 0;
 
@@ -382,15 +378,11 @@ bool Http2HpackDecoder::decode_headers(const uint8_t* encoded_headers,
             decoded_headers_size, MAX_OCTETS - decoded_headers_size, line_bytes_written);
         decoded_headers_size += line_bytes_written;
     }
-    else
-        decode_error = true;
+
     return success;
 }
 
-const Field* Http2HpackDecoder::get_decoded_headers(const uint8_t* const decoded_headers)
+Field Http2HpackDecoder::get_decoded_headers(const uint8_t* const decoded_headers)
 {
-    if (decode_error)
-        return new Field(STAT_NO_SOURCE);
-    else
-        return new Field(decoded_headers_size, decoded_headers, false);
+    return Field(decoded_headers_size, decoded_headers, false);
 }
diff --git a/src/service_inspectors/http2_inspect/http2_hpack.h b/src/service_inspectors/http2_inspect/http2_hpack.h
index 6f3badb8d..e7f2a5f6f 100644
--- a/src/service_inspectors/http2_inspect/http2_hpack.h
+++ b/src/service_inspectors/http2_inspect/http2_hpack.h
@@ -74,14 +74,13 @@ public:
 
     bool finalize_start_line();
     const Field* get_start_line();
-    const Field* get_decoded_headers(const uint8_t* const decoded_headers);
+    Field get_decoded_headers(const uint8_t* const decoded_headers);
     HpackIndexTable* get_decode_table() { return &decode_table; }
 
 private:
     Http2StartLine* start_line;
     bool pseudo_headers_allowed;
     uint32_t decoded_headers_size;
-    bool decode_error;
     Http2EventGen* const events;
     Http2Infractions* const infractions;
 
diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc
index 09f942f83..cdd35e94c 100644
--- a/src/service_inspectors/http2_inspect/http2_inspect.cc
+++ b/src/service_inspectors/http2_inspect/http2_inspect.cc
@@ -123,8 +123,7 @@ void Http2Inspect::eval(Packet* p)
         return;
 
     // FIXIT-E Workaround for unexpected eval() calls
-    // Avoid eval if scan/reassemble aborts
-    if (session_data->frame_type[source_id] == FT__ABORT or
+    if (session_data->abort_flow[source_id] or
         ((session_data->frame_header[source_id] == nullptr ) and
         (session_data->frame_data[source_id] == nullptr)))
     {
@@ -183,6 +182,7 @@ void Http2Inspect::clear(Packet* p)
     session_data->frame_in_detection = false;
 
     const SourceId source_id = p->is_from_client() ? SRC_CLIENT : SRC_SERVER;
+
     Http2Stream* stream = session_data->get_current_stream(source_id);
     stream->clear_frame();
     session_data->stream_in_hi = NO_STREAM_ID;
diff --git a/src/service_inspectors/http2_inspect/http2_request_line.cc b/src/service_inspectors/http2_inspect/http2_request_line.cc
index 8f7aada3a..ec56213d4 100644
--- a/src/service_inspectors/http2_inspect/http2_request_line.cc
+++ b/src/service_inspectors/http2_inspect/http2_request_line.cc
@@ -78,7 +78,7 @@ void Http2RequestLine::process_pseudo_header(const Field& name, const Field& val
 
 // This is called on the first non-pseudo-header. Select the appropriate URI form based on the
 // provided pseudo-headers and generate the start line
-bool Http2RequestLine::generate_start_line(const Field*& start_line)
+bool Http2RequestLine::generate_start_line(Field& start_line)
 {
     uint32_t bytes_written = 0;
 
@@ -188,7 +188,7 @@ bool Http2RequestLine::generate_start_line(const Field*& start_line)
     }
     else
     {
-        // FIXIT-L May want to be more lenient than RFC on generating start line
+        // FIXIT-E May want to be more lenient than RFC on generating start line
         *infractions += INF_PSEUDO_HEADER_URI_FORM_MISMATCH;
         events->create_event(EVENT_MISFORMATTED_HTTP2);
         return false;
@@ -198,7 +198,7 @@ bool Http2RequestLine::generate_start_line(const Field*& start_line)
     bytes_written += 2;
     assert(bytes_written == start_line_length);
 
-    start_line = new Field(start_line_length, start_line_buffer, false);
+    start_line.set(start_line_length, start_line_buffer, false);
 
     return true;
 }
diff --git a/src/service_inspectors/http2_inspect/http2_request_line.h b/src/service_inspectors/http2_inspect/http2_request_line.h
index 1b48eeb34..07294b438 100644
--- a/src/service_inspectors/http2_inspect/http2_request_line.h
+++ b/src/service_inspectors/http2_inspect/http2_request_line.h
@@ -30,7 +30,7 @@ class Http2RequestLine : public Http2StartLine
 {
 public:
     void process_pseudo_header(const Field& name, const Field& value) override;
-    bool generate_start_line(const Field*& start_line) override;
+    bool generate_start_line(Field& start_line) override;
 
     friend Http2StartLine* Http2StartLine::new_start_line_generator(HttpCommon::SourceId source_id,
         Http2EventGen* const events, Http2Infractions* const infractions);
diff --git a/src/service_inspectors/http2_inspect/http2_settings_frame.cc b/src/service_inspectors/http2_inspect/http2_settings_frame.cc
index bee617999..c8409ae9b 100644
--- a/src/service_inspectors/http2_inspect/http2_settings_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_settings_frame.cc
@@ -46,10 +46,10 @@ static uint32_t get_parameter_value(const uint8_t* data_buffer)
         data_buffer[frame_value_index + 3];
 }
 
-Http2SettingsFrame::Http2SettingsFrame(const uint8_t* header_buffer, const int32_t header_len,
-    const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* ssn_data,
-    HttpCommon::SourceId src_id, Http2Stream* stream_) : Http2Frame(header_buffer, header_len, data_buffer, data_len,
-    ssn_data, src_id, stream_)
+Http2SettingsFrame::Http2SettingsFrame(const uint8_t* header_buffer, const uint32_t header_len,
+    const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
+    HttpCommon::SourceId src_id, Http2Stream* stream_) : Http2Frame(header_buffer, header_len,
+    data_buffer, data_len, ssn_data, src_id, stream_)
 {
     if (!sanity_check())
     {
@@ -91,6 +91,7 @@ bool Http2SettingsFrame::sanity_check()
 {
     const bool ack = SfAck & get_flags();
 
+    // FIXIT-E this next check should possibly be moved to valid_sequence() 
     if (get_stream_id() != 0)
         bad_frame = true;
     else if (!ack and ((data.length() % 6) != 0))
diff --git a/src/service_inspectors/http2_inspect/http2_settings_frame.h b/src/service_inspectors/http2_inspect/http2_settings_frame.h
index ad910862e..544d91f75 100644
--- a/src/service_inspectors/http2_inspect/http2_settings_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_settings_frame.h
@@ -29,8 +29,8 @@ class Http2Frame;
 class Http2SettingsFrame : public Http2Frame
 {
 public:
-    friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const int32_t, const uint8_t*,
-        const int32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+    friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
     bool is_detection_required() const override { return false; }
 
 #ifdef REG_TEST
@@ -38,8 +38,8 @@ public:
 #endif
 
 private:
-    Http2SettingsFrame(const uint8_t* header_buffer, const int32_t header_len,
-        const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* ssn_data,
+    Http2SettingsFrame(const uint8_t* header_buffer, const uint32_t header_len,
+        const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
         HttpCommon::SourceId src_id, Http2Stream* stream);
 
     void parse_settings_frame();
diff --git a/src/service_inspectors/http2_inspect/http2_start_line.h b/src/service_inspectors/http2_inspect/http2_start_line.h
index a4dbff2e9..0e978bebe 100644
--- a/src/service_inspectors/http2_inspect/http2_start_line.h
+++ b/src/service_inspectors/http2_inspect/http2_start_line.h
@@ -43,7 +43,7 @@ public:
 
     friend class Http2Hpack;
 
-    virtual bool generate_start_line(const Field*& start_line) = 0;
+    virtual bool generate_start_line(Field& start_line) = 0;
     virtual void process_pseudo_header(const Field& name, const Field& value) = 0;
 
 protected:
diff --git a/src/service_inspectors/http2_inspect/http2_status_line.cc b/src/service_inspectors/http2_inspect/http2_status_line.cc
index f66fee8d3..2db7f52f8 100644
--- a/src/service_inspectors/http2_inspect/http2_status_line.cc
+++ b/src/service_inspectors/http2_inspect/http2_status_line.cc
@@ -53,7 +53,7 @@ void Http2StatusLine::process_pseudo_header(const Field& name, const Field& valu
 }
 
 // This is called on the first non-pseudo-header.
-bool Http2StatusLine::generate_start_line(const Field*& start_line)
+bool Http2StatusLine::generate_start_line(Field& start_line)
 {
     uint32_t bytes_written = 0;
 
@@ -80,7 +80,7 @@ bool Http2StatusLine::generate_start_line(const Field*& start_line)
     bytes_written += 2;
     assert(bytes_written == start_line_length);
 
-    start_line = new Field(start_line_length, start_line_buffer, false);
+    start_line.set(start_line_length, start_line_buffer, false);
 
     return true;
 }
diff --git a/src/service_inspectors/http2_inspect/http2_status_line.h b/src/service_inspectors/http2_inspect/http2_status_line.h
index 9b497089e..fd80b908a 100644
--- a/src/service_inspectors/http2_inspect/http2_status_line.h
+++ b/src/service_inspectors/http2_inspect/http2_status_line.h
@@ -29,7 +29,7 @@ class Http2StatusLine : public Http2StartLine
 {
 public:
     void process_pseudo_header(const Field& name, const Field& value) override;
-    bool generate_start_line(const Field*& start_line) override;
+    bool generate_start_line(Field& start_line) override;
 
     friend Http2StartLine* Http2StartLine::new_start_line_generator(HttpCommon::SourceId source_id,
         Http2EventGen* const events, Http2Infractions* const infractions);
diff --git a/src/service_inspectors/http2_inspect/http2_stream.cc b/src/service_inspectors/http2_inspect/http2_stream.cc
index 44ffc05b6..db2b66e29 100644
--- a/src/service_inspectors/http2_inspect/http2_stream.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream.cc
@@ -51,16 +51,24 @@ Http2Stream::~Http2Stream()
     delete data_cutter[SRC_SERVER];
 }
 
-void Http2Stream::eval_frame(const uint8_t* header_buffer, int32_t header_len,
-    const uint8_t* data_buffer, int32_t data_len, SourceId source_id)
+void Http2Stream::eval_frame(const uint8_t* header_buffer, uint32_t header_len,
+    const uint8_t* data_buffer, uint32_t data_len, SourceId source_id)
 {
     assert(current_frame == nullptr);
     current_frame = Http2Frame::new_frame(header_buffer, header_len, data_buffer,
         data_len, session_data, source_id, this);
-    valid_frame_order[source_id] = valid_frame_order[source_id] &&
-        current_frame->valid_sequence(state[source_id]);
-    current_frame->analyze_http1();
-    current_frame->update_stream_state();
+    if (!session_data->abort_flow[source_id] && (get_state(source_id) != STREAM_ERROR))
+    {
+        if (current_frame->valid_sequence(state[source_id]))
+        {
+            current_frame->analyze_http1();
+            current_frame->update_stream_state();
+        }
+        else
+        {
+            set_state(source_id, STREAM_ERROR);
+        }
+    }
 }
 
 void Http2Stream::clear_frame()
diff --git a/src/service_inspectors/http2_inspect/http2_stream.h b/src/service_inspectors/http2_inspect/http2_stream.h
index 33413687b..abf761939 100644
--- a/src/service_inspectors/http2_inspect/http2_stream.h
+++ b/src/service_inspectors/http2_inspect/http2_stream.h
@@ -37,8 +37,8 @@ public:
     Http2Stream(uint32_t stream_id, Http2FlowData* session_data_);
     ~Http2Stream();
     uint32_t get_stream_id() { return stream_id; }
-    void eval_frame(const uint8_t* header_buffer, int32_t header_len, const uint8_t* data_buffer,
-        int32_t data_len, HttpCommon::SourceId source_id);
+    void eval_frame(const uint8_t* header_buffer, uint32_t header_len, const uint8_t* data_buffer,
+        uint32_t data_len, HttpCommon::SourceId source_id);
     void clear_frame();
     const Field& get_buf(unsigned id);
     HttpFlowData* get_hi_flow_data() const { return hi_flow_data; }
@@ -78,7 +78,6 @@ public:
 private:
     const uint32_t stream_id;
     Http2FlowData* const session_data;
-    bool valid_frame_order[2] = { true, true };
     Http2Frame* current_frame = nullptr;
     HttpFlowData* hi_flow_data = nullptr;
     HttpMsgSection* hi_msg_section = nullptr;
diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter.cc b/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
index c92808aaa..72ed92337 100644
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
@@ -89,14 +89,14 @@ StreamSplitter::Status Http2StreamSplitter::scan(Packet* pkt, const uint8_t* dat
 #endif
 
     // General mechanism to abort using scan
-    if (session_data->frame_type[source_id] == FT__ABORT)
+    if (session_data->abort_flow[source_id])
         return HttpStreamSplitter::status_value(StreamSplitter::ABORT, true);
 
     const StreamSplitter::Status ret_val =
         implement_scan(session_data, data, length, flush_offset, source_id);
 
     if (ret_val == StreamSplitter::ABORT)
-        session_data->frame_type[source_id] = FT__ABORT;
+        session_data->abort_flow[source_id] = true;
 
 #ifdef REG_TEST
     if (HttpTestManager::use_test_input(HttpTestManager::IN_HTTP2))
@@ -151,6 +151,8 @@ const StreamBuffer Http2StreamSplitter::reassemble(Flow* flow, unsigned total, u
     }
 #endif
 
+    assert(!session_data->abort_flow[source_id]);
+
     // FIXIT-P: scan uses this to discard bytes until StreamSplitter:DISCARD
     // is implemented
     if (session_data->payload_discard[source_id])
@@ -191,6 +193,8 @@ bool Http2StreamSplitter::finish(Flow* flow)
     Http2FlowData* session_data = (Http2FlowData*)flow->get_flow_data(Http2FlowData::inspector_id);
     if (!session_data)
         return false;
+    if (session_data->abort_flow[source_id])
+        return false;
 
 #ifdef REG_TEST
     if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP2))
@@ -214,7 +218,7 @@ bool Http2StreamSplitter::finish(Flow* flow)
     for (const Http2FlowData::StreamInfo& stream_info : session_data->streams)
     {
         if ((stream_info.id == 0)                                                 ||
-            (stream_info.stream->get_state(source_id) == STREAM_COMPLETE)         ||
+            (stream_info.stream->get_state(source_id) >= STREAM_COMPLETE)         ||
             (stream_info.stream->get_hi_flow_data() == nullptr)                   ||
             (stream_info.stream->get_hi_flow_data()->get_type_expected(source_id)
                 != HttpEnums::SEC_BODY_H2))
@@ -251,7 +255,8 @@ bool Http2StreamSplitter::init_partial_flush(Flow* flow)
 
     Http2FlowData* session_data = (Http2FlowData*)flow->get_flow_data(Http2FlowData::inspector_id);
     assert(session_data != nullptr);
-    UNUSED(session_data); // just for a little while
+    if (session_data->abort_flow[source_id])
+        return false;
 
 #ifdef REG_TEST
     if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP2) &&
diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
index df08d4c00..540266e1f 100644
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -363,9 +363,8 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
         // This is the first reassemble() for this frame and we need to allocate some buffers
         session_data->frame_header_size[source_id] = FRAME_HEADER_LENGTH *
             session_data->num_frame_headers[source_id];
-        if (session_data->frame_header_size[source_id] > 0)
-            session_data->frame_header[source_id] =
-                new uint8_t[session_data->frame_header_size[source_id]];
+        session_data->frame_header[source_id] =
+            new uint8_t[session_data->frame_header_size[source_id]];
 
         session_data->frame_header_offset[source_id] = 0;
     }
@@ -374,7 +373,7 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
     {
         if (session_data->flushing_data[source_id])
         {
-            assert(total  > (FRAME_HEADER_LENGTH - 1));
+            assert(total > (FRAME_HEADER_LENGTH - 1));
             const uint32_t total_data = total - (FRAME_HEADER_LENGTH - 1);
             if (offset+len > total_data)
             {
@@ -392,10 +391,11 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
 
         if (len != 0)
         {
-            Http2DataCutter* data_cutter = stream->get_data_cutter(source_id);
+            Http2DataCutter* const data_cutter = stream->get_data_cutter(source_id);
             StreamBuffer http_frame_buf = data_cutter->reassemble(data, len);
             if (http_frame_buf.data)
             {
+                // FIXIT-L this use of const_cast is worrisome
                 session_data->frame_data[source_id] = const_cast<uint8_t*>(http_frame_buf.data);
                 session_data->frame_data_size[source_id] = http_frame_buf.length;
                 if (!session_data->flushing_data[source_id] && stream->is_partial_buf_pending(
diff --git a/src/service_inspectors/http2_inspect/http2_tables.cc b/src/service_inspectors/http2_inspect/http2_tables.cc
index 4a4808f9b..ae1fae5cf 100644
--- a/src/service_inspectors/http2_inspect/http2_tables.cc
+++ b/src/service_inspectors/http2_inspect/http2_tables.cc
@@ -50,6 +50,7 @@ const RuleMap Http2Module::http2_events[] =
     { EVENT_PSEUDO_HEADER_AFTER_REGULAR_HEADER, "HTTP/2 pseudo-header after regular header" },
     { EVENT_PSEUDO_HEADER_IN_TRAILERS, "HTTP/2 pseudo-header in trailers" },
     { EVENT_INVALID_PSEUDO_HEADER, "invalid HTTP/2 pseudo-header" },
+    { EVENT_TRAILERS_NOT_END, "HTTP/2 trailers without END_STREAM bit" },
     { 0, nullptr }
 };
 
diff --git a/src/service_inspectors/http_inspect/http_field.cc b/src/service_inspectors/http_inspect/http_field.cc
index 2fe479860..148d91ec1 100644
--- a/src/service_inspectors/http_inspect/http_field.cc
+++ b/src/service_inspectors/http_inspect/http_field.cc
@@ -39,6 +39,16 @@ Field::Field(int32_t length, const uint8_t* start, bool own_the_buffer_) :
     assert(!((start != nullptr) && (length < 0)));
 }
 
+Field& Field::operator=(const Field& rhs)
+{
+    assert(len == STAT_NOT_COMPUTE);
+    assert(strt == nullptr);
+    strt = rhs.strt;
+    len = rhs.len;
+    own_the_buffer = false;    // buffer must not have two owners
+    return *this;
+}
+
 void Field::set(int32_t length, const uint8_t* start, bool own_the_buffer_)
 {
     assert(len == STAT_NOT_COMPUTE);
diff --git a/src/service_inspectors/http_inspect/http_field.h b/src/service_inspectors/http_inspect/http_field.h
index 9d42f957c..57c656a7a 100644
--- a/src/service_inspectors/http_inspect/http_field.h
+++ b/src/service_inspectors/http_inspect/http_field.h
@@ -38,6 +38,7 @@ public:
     Field(int32_t length, const uint8_t* start, bool own_the_buffer_ = false);
     explicit Field(int32_t length) : len(length) { assert(length<=0); }
     Field() = default;
+    Field& operator=(const Field& rhs);
     ~Field() { if (own_the_buffer) delete[] strt; }
     int32_t length() const { return len; }
     const uint8_t* start() const { return strt; }
@@ -51,8 +52,6 @@ public:
 #endif
 
 private:
-    Field& operator=(const Field&) = delete;
-
     const uint8_t* strt = nullptr;
     int32_t len = HttpCommon::STAT_NOT_COMPUTE;
     bool own_the_buffer = false;