erlyaws / yaws

Yaws webserver
https://erlyaws.github.io
BSD 3-Clause "New" or "Revised" License
1.28k stars 267 forks source link

yaws_server:compressible_mime_type/2 crash #370

Closed leoliu closed 5 years ago

leoliu commented 5 years ago

In embedded Yaws (as started by yaws_api:embedded_start_conf/4), if deflate is turned on, a request will crash because delate_options is undefined.

Yaws process died: {function_clause,
                       [{yaws_server,compressible_mime_type,
                            ["text/html",undefined],
                            [{file, "/yaws/src/yaws_server.erl"},
                             {line,4868}]},
....
vinoski commented 5 years ago

I assume the patch below will fix this. Can you try it and let me know?

diff --git a/src/yaws_server.erl b/src/yaws_server.erl
index fc265aa..a2e987a 100644
--- a/src/yaws_server.erl
+++ b/src/yaws_server.erl
@@ -4865,6 +4865,8 @@ suffix_type(SC, L) ->
     end.

+compressible_mime_type(_Mime, undefined) ->
+    false;
 compressible_mime_type(Mime, #deflate{mime_types=MimeTypes}) ->
     case yaws:split_sep(Mime, $/) of
         [Type, SubType] -> compressible_mime_type(Mime,Type,SubType,MimeTypes);
leoliu commented 5 years ago

This handles the crash but it also ignores the deflate flag i.e. no compression.

vinoski commented 5 years ago

Perhaps this is a better patch:

diff --git a/src/yaws_server.erl b/src/yaws_server.erl
index fc265aa..1cbf8a2 100644
--- a/src/yaws_server.erl
+++ b/src/yaws_server.erl
@@ -4865,6 +4865,8 @@ suffix_type(SC, L) ->
     end.

+compressible_mime_type(Mime, undefined) ->
+    compressible_mime_type(Mime, #deflate{});
 compressible_mime_type(Mime, #deflate{mime_types=MimeTypes}) ->
     case yaws:split_sep(Mime, $/) of
     [Type, SubType] -> compressible_mime_type(Mime,Type,SubType,MimeTypes);
leoliu commented 5 years ago

But yaws_server:decide_deflate/7 still uses SC#sconf.deflate_options and crashes.

vinoski commented 5 years ago

I guess this patch is needed too:

diff --git a/src/yaws_server.erl b/src/yaws_server.erl
index fc265aa..e800add 100644
--- a/src/yaws_server.erl
+++ b/src/yaws_server.erl
@@ -3689,7 +3689,10 @@ decide_deflate(_, _, _, _, _, deflate, _) ->
     ?Debug("Compression already handled: Encoding=deflate~n", []),
     false;
 decide_deflate(true, SC, Arg, Sz, Data, decide, Mode) ->
-    DOpts = SC#sconf.deflate_options,
+    DOpts = case SC#sconf.deflate_options of
+                undefined -> #deflate{};
+                DOpts0 -> DOpts0
+            end,
     if
         Mode == final andalso size(Data) == 0 ->
             ?Debug("No data to be compressed~n",[]),
leoliu commented 5 years ago

That could fix the issue but I wonder if the following is a better fix i.e. by ensuring deflate_options is initialised properly:

diff --git a/src/yaws.erl b/src/yaws.erl
index 9487b87d..25b8c5ab 100644
--- a/src/yaws.erl
+++ b/src/yaws.erl
@@ -573,7 +573,9 @@ deflate_use_gzip_static  (D, Bool)  -> D#deflate{use_gzip_static   = Bool}.
 deflate_mime_types       (D, Types) -> D#deflate{mime_types        = Types}.

-setup_deflate(SL, DefaultDeflate) ->
+setup_deflate(SL, undefined, Flags) when Flags band ?SC_DEFLATE > 0 ->
+    setup_deflate(SL, #deflate{}, Flags);
+setup_deflate(SL, DefaultDeflate, _Flags) ->
     case lkup(deflate_options, SL, undefined) of
         undefined ->
             DefaultDeflate;
@@ -723,9 +725,9 @@ set_gc_flags([], Flags) ->

 %% Setup vhost configuration
 setup_sconf(SL, SC) ->
+    Flags = set_sc_flags(lkup(flags, SL, []), SC#sconf.flags),
     #sconf{port                  = lkup(port, SL, SC#sconf.port),
-           flags                 = set_sc_flags(lkup(flags, SL, []),
-                                                SC#sconf.flags),
+           flags                 = Flags,
            redirect_map          = lkup(redirect_map, SL,
                                         SC#sconf.redirect_map),
            rhost                 = lkup(rhost, SL, SC#sconf.rhost),
@@ -769,7 +771,7 @@ setup_sconf(SL, SC) ->
                                         SC#sconf.fcgi_app_server),
            php_handler           = lkup(php_handler, SL, SC#sconf.php_handler),
            shaper                = lkup(shaper, SL, SC#sconf.shaper),
-           deflate_options       = setup_deflate(SL, SC#sconf.deflate_options),
+           deflate_options       = setup_deflate(SL, SC#sconf.deflate_options, Flags),
            mime_types_info       = setup_mime_types_info(
                                      SL, SC#sconf.mime_types_info
                                     ),
vinoski commented 5 years ago

Yes, thanks, this is a proper fix. My earlier patches weren't meant as fixes, but to get a better idea of where things were going wrong.

Commit 68f27a2 with the fix is now on master.