Jeff King <p...@peff.net> writes:

> So something like this. It turned out to be a lot uglier than I had
> hoped because we get fed the data from curl in odd-sized chunks, so we
> need a state machine.

It is unfortunate that we have to snoop the protocol like this to
infer an error, but I do not think we can do better than that
approach.  FWIW, I did not find the logic in update_pktline_state()
you wrote ugly at all.

Having to assume that the end of each round from the other end must
be a FLUSH does feel somewhat ugly and brittle, though.

> diff --git a/remote-curl.c b/remote-curl.c
> index f14c41f4c..605357d77 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -403,6 +403,18 @@ struct rpc_state {
>       struct strbuf result;
>       unsigned gzip_request : 1;
>       unsigned initial_buffer : 1;
> +
> +     enum {
> +             RPC_PKTLINE_ERROR, /* bogus hex chars in length */
> +             RPC_PKTLINE_INITIAL, /* no packets received yet */
> +             RPC_PKTLINE_1, /* got one hex char */
> +             RPC_PKTLINE_2, /* got two hex chars */
> +             RPC_PKTLINE_3, /* got three hex chars */
> +             RPC_PKTLINE_DATA, /* reading data; pktline_len holds remaining 
> */
> +             RPC_PKTLINE_END_OF_PACKET, /* last packet completed */
> +             RPC_PKTLINE_FLUSH, /* last packet was flush */
> +     } pktline_state;
> +     size_t pktline_len;
>  };
>  
>  static size_t rpc_out(void *ptr, size_t eltsize,
> @@ -451,11 +463,77 @@ static curlioerr rpc_ioctl(CURL *handle, int cmd, void 
> *clientp)
>  }
>  #endif
>  
> +static void update_pktline_state(struct rpc_state *rpc,
> +                              const char *buf, size_t len)
> +{
> +#define READ_ONE_HEX(shift) do { \
> +     int val = hexval(buf[0]); \
> +     if (val < 0) { \
> +             warning("error on %d", *buf); \
> +             rpc->pktline_state = RPC_PKTLINE_ERROR; \
> +             return; \
> +     } \
> +     rpc->pktline_len |= val << shift; \
> +     buf++; \
> +     len--; \
> +} while(0)
> +
> +     while (len > 0) {
> +             switch (rpc->pktline_state) {
> +             case RPC_PKTLINE_ERROR:
> +                     /* previous error; there is no recovery */
> +                     return;
> +
> +             /* We can start a new pktline at any of these states */
> +             case RPC_PKTLINE_INITIAL:
> +             case RPC_PKTLINE_FLUSH:
> +             case RPC_PKTLINE_END_OF_PACKET:
> +                     rpc->pktline_len = 0;
> +                     READ_ONE_HEX(12);
> +                     rpc->pktline_state = RPC_PKTLINE_1;
> +                     break;
> +
> +             case RPC_PKTLINE_1:
> +                     READ_ONE_HEX(8);
> +                     rpc->pktline_state = RPC_PKTLINE_2;
> +                     break;
> +
> +             case RPC_PKTLINE_2:
> +                     READ_ONE_HEX(4);
> +                     rpc->pktline_state = RPC_PKTLINE_3;
> +                     break;
> +
> +             case RPC_PKTLINE_3:
> +                     READ_ONE_HEX(0);
> +                     if (rpc->pktline_len) {
> +                             rpc->pktline_state = RPC_PKTLINE_DATA;
> +                             rpc->pktline_len -= 4;
> +                     } else
> +                             rpc->pktline_state = RPC_PKTLINE_FLUSH;
> +                     break;
> +
> +             case RPC_PKTLINE_DATA:
> +                     if (len < rpc->pktline_len) {
> +                             rpc->pktline_len -= len;
> +                             len = 0;
> +                     } else {
> +                             buf += rpc->pktline_len;
> +                             len -= rpc->pktline_len;
> +                             rpc->pktline_len = 0;
> +                             rpc->pktline_state = RPC_PKTLINE_END_OF_PACKET;
> +                     }
> +                     break;
> +             }
> +     }
> +#undef READ_ONE_HEX
> +}
> +
>  static size_t rpc_in(char *ptr, size_t eltsize,
>               size_t nmemb, void *buffer_)
>  {
>       size_t size = eltsize * nmemb;
>       struct rpc_state *rpc = buffer_;
> +     update_pktline_state(rpc, ptr, size);
>       write_or_die(rpc->in, ptr, size);
>       return size;
>  }
> @@ -659,6 +737,8 @@ static int post_rpc(struct rpc_state *rpc)
>       curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
>       curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
>  
> +     rpc->pktline_state = RPC_PKTLINE_INITIAL;
> +
>       err = run_slot(slot, NULL);
>       if (err == HTTP_REAUTH && !large_request) {
>               credential_fill(&http_auth);
> @@ -667,6 +747,11 @@ static int post_rpc(struct rpc_state *rpc)
>       if (err != HTTP_OK)
>               err = -1;
>  
> +     if (rpc->pktline_state != RPC_PKTLINE_FLUSH) {
> +             error("invalid or truncated response from http server");
> +             err = -1;
> +     }
> +
>       curl_slist_free_all(headers);
>       free(gzip_body);
>       return err;

Reply via email to