cmake: add CUSTOM_HTTP_HEADERS for the automated minilex process #1499

jooncheol · 2019-02-19T13:26:53Z

For accessing the http headers that lws doesn't support to read
from the websocket server response, internal lextable for http
protocol parser has to be re-generated by manually modifying the
lextable-strings.h and the minilex tool.

This change does automate this minilex process which updates
lextable and adds relevant enums like WSI_TOKEN_XXX into public
header in build time in safe/easy manner without modifying any
original source code under the lib directory.

Example:
$ cmake .. -DCUSTOM_HTTP_HEADERS=X-Foo,X-Bar

This CMake option is available on Unix/Linux/Mac OS X and Windows
(MSYS/MinGW only)

Additional explanation for lws-team for better understanding

How it works:

cmake copies lextable-strings.h from lib dir to build dir
lower cased custom header is automatically added into lettable-strings.h in build dir by using bash/cat/sed/tr (this is going to be used by minilex in the build dir)
lextable-strings-custom.h is copied in the build dir and will be included by lib/roles/http/header.c
upper cased WSI_TOKEN_{HEADER} is automatically added into lws-http.h in the build dir by using tail/wc/tr/grep. it will be copied under include/libwebsockets under build dir.
compile minilex with updated lextable-strings.h in the build dir, write lextable.h in the build dir
compile again minilex using the updated lextable.h and lextable-strings.h in the build dir, write the lextable-custom.h so the lib/roles/http/server/parsers.c can use it instead of original in the build time.
set LWS_WITH_CUSTOM_HTTP_HEADER in lws-config.h so Application can check wether the custom headers is available or not. #ifdef LWS_WITH_CUSTOM_HTTP_HEADER -> use WSI_TOKEN_XXX in the code

Since this PR uses standard unix tools like grep/tr/head/sed for automation, it's not available option for the VS project. But it's available for the windows using the MSYS/MinGW.

I've tested this PR on Mac OS X Mojave, Ubuntu 18.04, Windows MSYS2 (with MinGW) and worked well.

I believe this helps 2 cases

a lws user who want to handle the custom http header from the web socket server response.
- lws user can avoid the LGPL license violation since this doesn't touch the original source code. custom header supported is added as part of build process (for me, this is the reason why I started the PR)
- lws user can avoid the mistake whenever they need to use the current minilex process.
the lws-team whenever needs to add standard http header into the lextable.
- use this Cmake option then update lextable.h/lextable-strings.h under lib dir from -custom.h in the build dir. => this will reduce the mistake when the lextable needs to add new standard http header

For accessing the http headers that lws doesn't support to read from the websocket server response, internal lextable for http protocol parser has to be re-generated by manually modifying the lextable-strings.h and the minilex tool. This change does automate this minilex process which updates lextable and adds relevant enums like WSI_TOKEN_XXX into public header in build time in safe/easy manner without modifying any original source code under the lib directory. Example: cmake .. -DCUSTOM_HTTP_HEADERS=X-Foo,X-Bar This CMake option is available on Unix/Linux/Mac OS X and Windows (MSYS/MinGW only)

lws-team · 2019-02-19T15:18:20Z

Thanks... I appreciate what you're trying to do, but I don't really want to do this. There are a few reasons, some to do with the implementation:

Does it work when building cross?
There should be some way to use cmake build host targets to express the build action.
Cmake has platform independent cp at least.
There should be a away to move the sed stuff into C.
This build-time thing can't work with code linking against eg common distro libs built without the desired headers

There's often an existing header that can take the data in a standardized way (eg, whatever weirdo authentication token, it can use Authorization: with some other token than Basic). Sometimes though the peer already decided to make up its own random headers and you just have to follow along to interoperate.

Lws takes the approach that if it's not one of the 100 or so headers it understands, it just drops the header content... the good way to deal with this I think is add stuff to the ah parser to make it keep the unknown header content and eg, reserve a few extra ah pointers to point to say the first 4 unknown headers it met (it will also need to be able to "point" to the header name part and know its length). Then similar to how you can create headers by index or name, add apis to return any 'unknown' header content if any by header name. This'd have to be implemented on both the h1 and h2 ah parsers... although quite different they use the same ah struct. For h2 dynamic indexed headers, you'd have to copy the indexed header name out of the index and into the ah headers region and point to it at parse time, because the dynamic index can change inbetween parsing and querying.

That'll allow runtime extensible headers while keeping the current arrangements for common headers.

jooncheol · 2019-02-28T15:54:23Z

Hi Andy,

Thanks for the comment for the PR.
I believe the suggestion you've mentioned regarding h1/h2 parser is absolutely the right direction for the API requirement I wanted to bring.
But it doesn't look the level of change that I can try to contribute. I don't have enough understand the logic in there :-) I wish you add it into the plan and officially supported in the future.

BTW, I've updated PR. If you are ok, can you please review the update on this PR again?
I've tried to fix all the concerns you've raised in this approach. I wish this can be merged and available until your right direction comes to the master someday.

The usage is same... add -DCUSTOM_HTTP_HEADERS=X-Foo,X-Bar in the cmake time and the custom header support is automatically applied while building. I tried to use the CMake macro instead of the previous bash command.. extended the minilex.c to get rid of the bash command in previous PR.

here's update on your comment.

Does it work when building cross?

There should be some way to use cmake build host targets to express the build action.

Now, the change has considered cross compilation.
in build minilex is compiled by independent cmake.

Cmake has platform independent cp at least.

applied cmake -E copy

There should be a away to move the sed stuff into C.

added command line options into minilex.c. and replaced previous bash commands by minilex.
please see below.

$ ./minilex --help
./minilex - update lextable.h or others as per option
Usage:
  ./minilex [options] > output-file

Options
  --help or -h                   Print this help
  --lextable-strings headernames Update lextable-strings.h for given header names
  --wsi-token headernames        Update WSI_TOKEN in lws-http.h for given header names
  --in                           Input file

Example:
  ./minilex --lextable-strings X-Foo --in lextable-strings.h> lextable-strings.h
  ./minilex --wsi-token X-Foo --in ./lws-http.h > lws-http.h
  ./minilex > lextable.h    # update lextable.h
  ./minilex > lextable.h    # !!! should be run twice
$

This build-time thing can't work with code linking against eg common distro libs built without the desired headers

I'm not sure if I get your point correctly. but this PR defines LWS_WITH_CUSTOM_HTTP_HEADER in lws-config.h the option is given.
so, application can use the macro to check the custom header availablity like this

#ifdef LWS_WITH_CUSTOM_HTTP_HEADER
   // application build has picked libwebsockets build with custom header support
   // by specifying PKG_CONFIG_PATH={their .pc path} or from the custom prefix like /usr/local/include ...
   // so, it can use WSI_TOKEN_X_FOO, WSI_TOKEN_X_BAR in this case
#else 
  // application build has picked libwebsockets.h from the typical include path where the linux distro has like /usr/include ...
  // custom WST_TOKEN_ is not available in this case... application build can handle accordingly
#endif

…mpile

…hould be given

…lt ws binding On lwsws, incoming ws connections to the default vhost are not rejected by the dummy protocol handler and not really serviced either, leading to bots connecting to it to get immortal, idle ws connections with no timeout (since it's an established ws connection). Rejecting these connections by default by adding a handler for ESTABLISHED in the dummy handler will solve it nicely, but it will break an unknown number of dumb. protocol-less user implementations that rely on this behaviour by using break; from their own ESTABLISHED handler and calling through to the currently NOP dummy handler one. Add support to assertively disable the default protocol index used for subprotocol-less ws connections instead.

With SMP as soon as we add the new sockfd to the fds table, in the case we load-balanced the fd on to a different pt, service on it becomes live immediately and concurrently. This can lead to the unexpected situation that while we think we're still initing the new wsi from our thread, it can have lived out its whole life concurrently from another service thread. Add a volatile flag to inform the owning pt that if it wants to service the wsi during this init window, it must wait and retry next time around the event loop.

MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting m to 0 by default will prevent "error: ‘m’ may be used uninitialized in this function" while compiling with the option -DLWS_WITH_LIBUV=ON.

This seen in the wild... ==20578== Invalid read of size 1 ==20578== at 0x4D2E018: uv_poll_stop (poll.c:112) ==20578== by 0x48BC159: elops_io_uv (libuv.c:684) ==20578== by 0x4872F55: __remove_wsi_socket_from_fds (pollfd.c:326) ==20578== by 0x486EF1B: __lws_close_free_wsi (close.c:425) ==20578== by 0x486F3E2: lws_close_free_wsi (close.c:518) ==20578== by 0x487564C: lws_service_fd_tsi (service.c:1033) ==20578== by 0x48BAEA9: lws_io_cb (libuv.c:117) ==20578== by 0x4D3606F: uv__io_poll (linux-core.c:379) ==20578== by 0x4D27714: uv_run (core.c:361) ==20578== by 0x48BC347: elops_run_pt_uv (libuv.c:735) ==20578== by 0x4875746: lws_service (service.c:1080) ==20578== by 0x401A51: main (main.c:309) ==20578== Address 0x58 is not stack'd, malloc'd or (recently) free'd

The cmake config on the build system actually decides the release build optimization policy. On Fedora, it's -O2. On Ubuntu, it's -O3. Anything given in CMakeLists.txt is overridden by the build system policy since it goes at the end of the compiler commandline. When you are building cross, the build system's opinion of your cross binary optimization level is irrelevant, and at worst destructive. Some versions of gcc contain broken optimizations that are applied only at -O3. This patch removes any doomed attempt to set -O in CMakeLists.txt, which has no effect since the build system policy is still added at the end, but removes confusion; and adds code to all the cross build files to forcibly override release optimization level to -O2, removing the build system's opinion of how your cross build should look.

jooncheol · 2019-02-28T16:50:01Z

just pushed again with some fix like usage string of minilex, cmake dependency.
tested on OSX Mojave, Ubuntu 18.04 LTS, MSYS2

Until now lws only parses headers it knows at build-time from its prebuilt lexical analyzer. This adds an on-by-default cmake option and a couple of apis to also store and query "custom", ie, unknown-to-lws headers. A minimal example is also provided. At the moment it only works on h1, h2 support needs improvements to the hpack implementation. Since it bloats ah memory usage compared to without it if custom headers are present, the related code and ah footprint can be disabled with the cmake option LWS_WITH_CUSTOM_HEADERS, but it's on by default normally. ESP32 platform disables it. #1499

lws-team · 2019-02-28T23:22:13Z

I believe the suggestion you've mentioned regarding h1/h2 parser is absolutely the right direction for the API requirement I wanted to bring.

Great.

But it doesn't look the level of change that I can try to contribute. I don't have enough understand the logic in there :-) I wish you add it into the plan and officially supported in the future.

I have a patch for it on h1 already. Doing it on h2 is going to need a larger rework of the hpack stuff to improve that first. I pushed the h1 support on master along with a minimal example, please give it a try.

Until now lws only parses headers it knows at build-time from its prebuilt lexical analyzer. This adds an on-by-default cmake option and a couple of apis to also store and query "custom", ie, unknown-to-lws headers. A minimal example is also provided. At the moment it only works on h1, h2 support needs improvements to the hpack implementation. Since it bloats ah memory usage compared to without it if custom headers are present, the related code and ah footprint can be disabled with the cmake option LWS_WITH_CUSTOM_HEADERS, but it's on by default normally. ESP32 platform disables it. #1499

jooncheol · 2019-03-07T11:18:54Z

Hi Andy,

the custom headers support for h1 you've added (f5e5c59) works well.
Thanks again!

Until now lws only parses headers it knows at build-time from its prebuilt lexical analyzer. This adds an on-by-default cmake option and a couple of apis to also store and query "custom", ie, unknown-to-lws headers. A minimal example is also provided. At the moment it only works on h1, h2 support needs improvements to the hpack implementation. Since it bloats ah memory usage compared to without it if custom headers are present, the related code and ah footprint can be disabled with the cmake option LWS_WITH_CUSTOM_HEADERS, but it's on by default normally. ESP32 platform disables it. #1499

lws-team and others added 3 commits February 18, 2019 09:02

server-status: add proc statm and switch to vhost protocol timer

2b5c8ef

socks5: fix remain dst buffer length when calls lws_strncpy

afbf40c

lws-team force-pushed the master branch from 7229678 to c36fcae Compare February 22, 2019 21:56

jooncheol force-pushed the customheader branch 2 times, most recently from fb11f27 to b321a07 Compare February 28, 2019 15:22

jooncheol and others added 10 commits February 28, 2019 16:41

minilex, cmake: support custom http header for all platforms&cross co…

6189ffe

…mpile

server-status: add proc statm and switch to vhost protocol timer

032fac0

http: log ws upgrades

943efef

clean: LWS_SSL_ENABLED use vh as the macro arg name to clarify what s…

3912aea

…hould be given

libuv.c: set m to 0 by default

03248ef

MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting m to 0 by default will prevent "error: ‘m’ may be used uninitialized in this function" while compiling with the option -DLWS_WITH_LIBUV=ON.

smp: take pt lock in poll foreign thread detection

a75f4f2

jooncheol force-pushed the customheader branch from b321a07 to 4a72990 Compare February 28, 2019 16:47

lws-team force-pushed the master branch 4 times, most recently from 3fe1966 to a4a701b Compare March 4, 2019 23:11

lws-team force-pushed the master branch from a4a701b to ecb2218 Compare March 5, 2019 07:48

jooncheol closed this Mar 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmake: add CUSTOM_HTTP_HEADERS for the automated minilex process #1499

cmake: add CUSTOM_HTTP_HEADERS for the automated minilex process #1499

jooncheol commented Feb 19, 2019

lws-team commented Feb 19, 2019

jooncheol commented Feb 28, 2019 •

edited

jooncheol commented Feb 28, 2019 •

edited

lws-team commented Feb 28, 2019

jooncheol commented Mar 7, 2019 •

edited

cmake: add CUSTOM_HTTP_HEADERS for the automated minilex process #1499

cmake: add CUSTOM_HTTP_HEADERS for the automated minilex process #1499

Conversation

jooncheol commented Feb 19, 2019

lws-team commented Feb 19, 2019

jooncheol commented Feb 28, 2019 • edited

jooncheol commented Feb 28, 2019 • edited

lws-team commented Feb 28, 2019

jooncheol commented Mar 7, 2019 • edited

jooncheol commented Feb 28, 2019 •

edited

jooncheol commented Feb 28, 2019 •

edited

jooncheol commented Mar 7, 2019 •

edited