Skip to content

Commit

Permalink
Fix h2_client:send_request/3
Browse files Browse the repository at this point in the history
h2_client:send_request/3 does not check the return from
h2_connection:new_stream/1,2. When a new stream cannot be created,
for example, because the peer connection sets the maximum concurrent
streams to 1, h2_connection:new_stream/1,2 returns {error, 7}.

However, because h2_client:send_request/3 does not check this,
it returns {ok, {error, 7}} to the caller, which blows up when it next
tries to use a stream id of {error, 7}.

This commit corrects this so that h2_client:send_request/3 returns
either {ok, stream_id()} or {error, error_code()}.

This commit also refactors sync_request/3 to call send_request/3, and
adds or corrects some type specs (such as the one for
h2_connection:new_stream/1).
  • Loading branch information
Edwin Fine committed Jan 19, 2017
1 parent adb3474 commit 15250e8
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 14 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ _build
rebar3
.rebar3
log
.*.sw?
38 changes: 26 additions & 12 deletions src/h2_client.erl
Original file line number Diff line number Diff line change
Expand Up @@ -114,21 +114,35 @@ start_ssl_upgrade_link(Host, Port, InitialMessage, SSLOptions) ->
stop(Pid) ->
h2_connection:stop(Pid).

-spec sync_request(CliPid, Headers, Body) -> Result when
CliPid :: pid(), Headers :: hpack:headers(), Body :: binary(),
Result :: {ok, {hpack:headers(), iodata()}}
| {error, error_code() | timeout}.
sync_request(CliPid, Headers, Body) ->
StreamId = h2_connection:new_stream(CliPid),
h2_connection:send_headers(CliPid, StreamId, Headers),
h2_connection:send_body(CliPid,StreamId,Body),
receive
{'END_STREAM', StreamId} ->
h2_connection:get_response(CliPid, StreamId)
after 5000 ->
{error, timeout}
case send_request(CliPid, Headers, Body) of
{ok, StreamId} ->
receive
{'END_STREAM', StreamId} ->
h2_connection:get_response(CliPid, StreamId)
after 5000 ->
{error, timeout}
end;
Error ->
Error
end.

-spec send_request(CliPid, Headers, Body) -> Result when
CliPid :: pid(), Headers :: hpack:headers(), Body :: binary(),
Result :: {ok, stream_id()} | {error, error_code()}.
send_request(CliPid, Headers, Body) ->
StreamId = h2_connection:new_stream(CliPid),
h2_connection:send_headers(CliPid, StreamId, Headers),
h2_connection:send_body(CliPid,StreamId,Body),
{ok, StreamId}.
case h2_connection:new_stream(CliPid) of
{error, _Code} = Err ->
Err;
StreamId ->
h2_connection:send_headers(CliPid, StreamId, Headers),
h2_connection:send_body(CliPid, StreamId, Body),
{ok, StreamId}
end.

-spec get_response(pid(), stream_id()) ->
{ok, {hpack:header(), iodata()}}
Expand Down
4 changes: 2 additions & 2 deletions src/h2_connection.erl
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ get_peercert(Pid) ->
is_push(Pid) ->
gen_fsm:sync_send_all_state_event(Pid, is_push).

-spec new_stream(pid()) -> stream_id().
-spec new_stream(pid()) -> stream_id() | {error, error_code()}.
new_stream(Pid) ->
new_stream(Pid, self()).

Expand All @@ -274,7 +274,7 @@ send_promise(Pid, StreamId, NewStreamId, Headers) ->

-spec get_response(pid(), stream_id()) ->
{ok, {hpack:headers(), iodata()}}
| {error, term()}.
| not_ready.
get_response(Pid, StreamId) ->
gen_fsm:sync_send_all_state_event(Pid, {get_response, StreamId}).

Expand Down

0 comments on commit 15250e8

Please sign in to comment.