1. 28 Nov, 2019 2 commits
  2. 26 Nov, 2019 5 commits
    • Emmanuel Hocdet's avatar
      Simplify WS allocation in tlv_string · e74f9e87
      Emmanuel Hocdet authored
      Patch by @ehocdet, commit message edited by @nigoroll:
      
      The root cause of #3131 was misdiagnosed to the extent that, while this
      change had prevented it, the root cause was a bug in WS_ReserveSize()
      fixed in 505b7bd9
      
      The previous tlv_string() code was correct except for the
      fact that error handling should have checked for WS_ReserveSize(ctx->ws,
      len+1) <= len (also spotted by @ehocdet).
      
      Someone had mentioned at some point that we would not want to VRT_fail(),
      but I think this must have been related to the proxy transport code, not
      the proxy vmod.
      
      Ref #3131
      e74f9e87
    • Nils Goroll's avatar
      Add Session Attribute workspace overflow handling · 287dc4a6
      Nils Goroll authored
      Notes:
      
      * for the acceptor, I think it makes sense to keep AN assertion (pun!)
        because varnish is not viable if the session workspace is too small
        to even hold the attributes initialized in the acceptor.
      
        If this was an issue, we should rather revisit the minimum values for
        the session workspace
      
      * for h1 and h2 session setup, I have used XXXAN() because I am not sure
        how we should best handle allocation failures.
      
      * The relevant bit, for now, is the proxy code which may allocate
        arbitrarily long TLV attributes, so this is the code for which we now
        actually handle errors and test that we do
      
      On the vtc: I added the test to o00005.vtc because there existed a
      previous overflow test from 267504b8,
      but that only tested for the one case of a WS overflow which was already
      handled.
      
      Fixes #3145
      287dc4a6
    • Nils Goroll's avatar
      fix copy-pasta vtc description · 815331b3
      Nils Goroll authored
      815331b3
    • Nils Goroll's avatar
      WS_ReserveSize() must not hold a reservation for zero return value · 505b7bd9
      Nils Goroll authored
      This originates from a3d47c25, but
      was overlooked in 4e333597:
      
      When there is insufficient space to fulfil the reservation request, we
      must not leave the workspace reserved.
      
      Fixes #3131
      505b7bd9
    • Nils Goroll's avatar
      add a facility to test WS_ReserveSize() · ed3b095c
      Nils Goroll authored
      ed3b095c
  3. 25 Nov, 2019 1 commit
  4. 22 Nov, 2019 5 commits
  5. 20 Nov, 2019 1 commit
  6. 19 Nov, 2019 2 commits
    • Dridi Boukelmoune's avatar
      Put some serious red tape on VCL_STRANDS · 11d55148
      Dridi Boukelmoune authored
      I started suspecting that we need this clarification during the review
      of #3123 [1] and was able to verify it with a simple test case. First I
      needed a function I put in vmod_debug:
      
          $Function STRANDS hoard_strands(PRIV_TASK, STRANDS s)
      
          Return the first value of s for the rest of the task.
      
      The implementation is very straightforward:
      
          struct hoarder {
                 VCL_STRANDS     s;
          };
      
          VCL_STRANDS
          xyzzy_hoard_strands(VRT_CTX, struct vmod_priv *priv, VCL_STRANDS s)
          {
                 struct hoarder *h;
      
                 CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
                 AN(priv);
      
                 if (priv->priv == NULL) {
                         h = malloc(sizeof *h);
                         AN(h);
                         h->s = s;
                         priv->priv = h;
                         priv->free = free;
                 }
      
                 return (priv->priv);
          }
      
      And then the following test case results in a panic on my system, but I
      suspect this is generally undefined behavior and other nasty results may
      occur under different circumstances:
      
          varnishtest "Beware of STRANDS"
      
          varnish v1 -vcl {
                  import debug;
                  backend be none;
                  sub vcl_recv {
                          debug.hoard_strands("recv: " + req.url);
                  }
                  sub vcl_miss {
                          debug.hoard_strands("miss: " + req.url);
                          return (synth(200));
                  }
                  sub vcl_synth {
                          set resp.body = debug.hoard_strands("synth: " + req.url);
                          return (deliver);
                  }
          } -start
      
          client c1 {
                  txreq
                  rxresp
                  expect resp.body ~ recv
          } -run
      
      This also begs the following question: can it ever be safe to let a VMOD
      function return a STRANDS? Maybe it should be banned from return types.
      
      [1] https://github.com/varnishcache/varnish-cache/pull/3123#discussion_r345617108
      11d55148
    • Dridi Boukelmoune's avatar
      Why-does-slink-hate-capitalization OCD · 34261afd
      Dridi Boukelmoune authored
      Is it because German capitalizes so many words that his shift keys have
      become hit or miss?
      34261afd
  7. 18 Nov, 2019 10 commits
  8. 17 Nov, 2019 1 commit
  9. 16 Nov, 2019 3 commits
  10. 15 Nov, 2019 2 commits
    • Dridi Boukelmoune's avatar
      Make sure to use none backends in generated C code · 3ff76286
      Dridi Boukelmoune authored
      Otherwise you might run into this:
      
          Message from VCC-compiler:
          Unused backend nil, defined:
          ('<vcl.inline>' Line 4 Pos 17)
                  backend nil none;
          ----------------###------
      
          (That was just a warning)
          Message from C-compiler:
          vgc.c:1476:20: error: unused variable 'vgc_backend_nil' [-Werror,-Wunused-variable]
          static VCL_BACKEND vgc_backend_nil;
                             ^
          1 error generated.
          Running C-compiler failed, exited with 1
          VCL compilation failed
      
      This is done in both init and discard code to maintain the balance.
      3ff76286
    • Dridi Boukelmoune's avatar
      Revert "Correct WS allocation in tlv_string" · b9eaa943
      Dridi Boukelmoune authored
      This reverts commit 1d23a733.
      b9eaa943
  11. 14 Nov, 2019 4 commits
    • Emmanuel Hocdet's avatar
      Correct WS allocation in tlv_string · 1d23a733
      Emmanuel Hocdet authored
      Fixes #3131
      1d23a733
    • Dridi Boukelmoune's avatar
      GC stale check · 85fca27b
      Dridi Boukelmoune authored
      85fca27b
    • Dridi Boukelmoune's avatar
      We can't have fun outside the ISO C99 standard · 2215057d
      Dridi Boukelmoune authored
      Because someone brought a suncc to a vtest party.
      2215057d
    • Dridi Boukelmoune's avatar
      Big bang rewrite of m00049.vtc · 175af0b9
      Dridi Boukelmoune authored
      Now that vtc.typesize() is a bit more reliable, it's easier to deal with
      architecture differences for the sizeof struct vrt_blob. For a much more
      reliable failure check, it now looks at logs.
      
      There may also have been a time when depending on the error we trigger
      we could fail with a 500 or a 503, this is no longer the case.
      
      For the nitty-gritty of the new test case, there are some comments to
      hopefully help (at least my future self) decipher this beast.
      
      This new test case remains stable even with the changes from #3123.
      175af0b9
  12. 13 Nov, 2019 4 commits