Github user jeking3 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1093#discussion_r80496721
  
    --- Diff: lib/php/lib/Thrift/Protocol/TBinaryProtocol.php ---
    @@ -246,6 +257,10 @@ public function readMessageBegin(&$name, &$type, 
&$seqid)
           }
         }
     
    +    if ($seqid != $this->seqid_) {
    +      throw new TApplicationException("TBinaryProtocol::ReadMessageBegin 
received SequenceID: $seqid not matches requested ID: $this->seqid_ " . 
TApplicationException::BAD_SEQUENCE_ID);
    +    }
    --- End diff --
    
    I would suggest that the issue lies in the server implementation in PHP 
based on your description.  If a client can disconnect, then a new client 
connects and it receives the response from the server originally intended for 
the client that disconnected then the server is misbehaving and a massive 
security hole.  Perhaps it keeps a list of responses based on IP address of 
something (I haven't looked).  In any case, if you look at the C++ server, each 
connection is handled by a single thread, so there can only be one outstanding 
request at a time.  If the client disconnects, the thread completes the request 
and then fails to send the reply, and ends.  There is no possibility that 
another client would receive the reply.  I would recommend instead of trying to 
solve the issue on the client side that fixing the root cause on the server 
side would be better as it would be much more secure.  I don't like the notion 
that someone can connect to a thrift PHP server and possibly receive con
 fidential information originally destined for another connection.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to