Add custom headers with websocket request

[xiaotianrandom]

This is a patch to resolve #116 . It parses custom headers from content of a websocket tag with type connect, and send them with the handshake request. For example, we can send custom cookie using the following config:

<request subst="true">
  <websocket type="connect" path="/path/to/ws">
    Cookie: sessionid=%%_sessionid%%
  </websocket>
</request>

I'm new to Erlang. Could any maintainer give me some feedback? If the design is acceptable, I'd like to complete the document.

[tisba]

Hey @xiaotianrandom. Nice job! :)

One remark regarding the XML configuration. I would suggest, we keep it similar to HTTP headers:

<http url="/bla" method="GET">
  <www_authenticate userid="Aladdin" passwd="open sesame"/>
  <http_header name="Cache-Control" value="no-cache"/>
  <http_header name="Referer" value="http://www.w3.org/"/>
</http>

So basicially this would translate to:

<request subst="true">
  <websocket type="connect" path="/path/to/ws">
    <http_header name="…" value="…"/>
  </websocket>
</request>

Since the initial handshake is plain HTTP, this would also be better, if things like www_authenticate and a like would be added.

What do you think?

[xiaotianrandom]

@tisba , thanks for your response.

I agree with you that it's better to use http_header tags instead of raw content, and I actually tried this before. But websocket tags originally contains #PCDATA instead of child tags. To keep it similar to HTTP headers, I changed the dtd as follows:

diff --git a/tsung-1.0.dtd b/tsung-1.0.dtd
index 19ae935..e70b75d 100644
--- a/tsung-1.0.dtd
+++ b/tsung-1.0.dtd
@@ -374,7 +374,7 @@ repeat | if | change_type | foreach | set_option | interaction | abort )*>
     certfile    CDATA   #IMPLIED
     dn          CDATA   #IMPLIED
     >
-<!ELEMENT websocket (#PCDATA) >
+<!ELEMENT websocket (#PCDATA | http_header) >
 <!ATTLIST websocket
     type (connect | message | close) #REQUIRED
     ack  (no_ack | parse) #IMPLIED

And tsung complains:

3215- fatal: {illegal_for_mixed_content,once}
Config Error, aborting ! {fatal,
                             {{illegal_for_mixed_content,once},
                              {file,
                                  "/home/toby/.local/share/tsung/tsung-1.0.dtd"},
                              {line,377},
                              {col,45}}}

Is there an easy way to fix this?

[xiaotianrandom]

@tisba I figured out how to accept both PCDATA and children in an element in DTD. Now the patch reads user custom headers from http_header elements as you suggested:

<request subst="true">
  <websocket type="connect" path="/path/to/ws">
    <http_header name="Cookie" value="sessionid=%%_sessionid%%"/>
  </websocket>
</request>

I squashed all commits in this PR into one, and pushed --force.

[tisba]

I like the configuration part, this is much better!

I currently have very little time to test the changes thoroughly, but maybe @nniclausse can take a look? What do you think about this change?

[nniclausse]

Hi. it seems fine.

[xiaotianrandom]

Is there anything I can do to make this PR merged?

[tisba]

I've taken your branch for a little test and it seems to work just fine, thanks!

Could you maybe add a little remark to the documentation that we now can pass HTTP headers when connecting? Maybe under https://github.com/processone/tsung/blob/develop/docs/conf-sessions.rst (http://tsung.erlang-projects.org/user_manual/conf-sessions.html?highlight=websocket#websocket).

[xiaotianrandom]

I added a paragraph in the doc for this patch. Do I need to add a "New in x.x.x" mark? I use 1.7.1, since the latest version is 1.7.0 now.

[tisba]

Good question about the documentation of Tsung version :) @nniclausse: How do we want to handle hints for features in future Tsung versions? I'll merge this now as is and I'm happy to change the version remark in a follow up.

As for the code itself: 😎! Thank you for your contribution, @xiaotianrandom!

[jesseshieh]

Hi @tisba and @xiaotianrandom, thanks for this awesome enhancement! I tried to use it though and noticed that it was merged into the develop branch and wasn't included in the latest 1.7.0 release. I was just wondering if that was intended or not. Is there a plan to get this included in the next release? Also, what is the purpose of the develop branch?

[tisba]

@jesseshieh the 1.7.0 release is quite old already (last year). I started an issue for releasing 1.8.0 but I cannot really find time currently to drive this forward :-/

[jesseshieh]

Oh I'm so sorry, I misread the year! Sorry about that :)

[pskl]

Thanks @xiaotianrandom I hope this gets released soon 🙏

[tisba]

I'm guessing that a new "official" release will take some time.

But tsung is actually quite simple to build from source.