Merge pull request #1375 from ShaneMcGovernIE/fix/postlog-pipe-mode

src/core/HostShell.lua

@@ -428,10 +428,29 @@ function HostShell.httpPost(url, body, contentType, userAgent, maxTime)
   if type(body) ~= "string" then return nil, "missing body" end
   userAgent = userAgent or "gen1recomp"
   if HostShell.haveCurl() then
-    -- --data-binary @- keeps the payload out of argv (command-line length
+    -- io.popen is one-way on Lua/LuaJIT: its mode is "r" or "w", never
+    -- "rw".  Stage the request body so the response can stay on a read pipe.
+    local bodyPath = os.tmpname()
+    local bodyFile, bodyOpenErr = io.open(bodyPath, "wb")
+    if not bodyFile then
+      pcall(os.remove, bodyPath)
+      return nil, "could not create request body: " .. tostring(bodyOpenErr)
+    end
+    local bodyOk, bodyErr = pcall(function()
+      assert(bodyFile:write(body))
+      assert(bodyFile:close())
+    end)
+    if not bodyOk then
+      pcall(function() bodyFile:close() end)
+      pcall(os.remove, bodyPath)
+      return nil, "could not write body: " .. tostring(bodyErr)
+    end
+
+    -- --data-binary @<file> keeps the payload out of argv (command-line length
     -- limits on Windows) and preserves every byte including trailing
-    -- newlines.  No -f, matching httpGet: the response body is discarded
-    -- anyway, and curl's stderr carries the real diagnosis on failure.
+    -- newlines.  The body is staged above because io.popen cannot be opened
+    -- for both writing and reading.  No -f, matching httpGet: the response
+    -- body is discarded anyway, and curl's stderr carries the diagnosis.
     local cmd = ("curl -sSL --proto =http,https --proto-redir =http,https "
       .. "--connect-timeout 10 --max-time %d ")
       :format(tonumber(maxTime) or 40)
@@ -441,18 +460,17 @@ function HostShell.httpPost(url, body, contentType, userAgent, maxTime)
       cmd = cmd .. "-H " .. HostShell.quote("Content-Type: " .. contentType) .. " "
     end
     cmd = cmd .. "-H " .. HostShell.quote("Content-Length: " .. tostring(#body)) .. " "
-      .. "--data-binary @- "
+      .. "--data-binary " .. HostShell.quote("@" .. bodyPath) .. " "
       .. "-w " .. HostShell.quote(HTTP_MARK_FMT) .. " "
       .. HostShell.quote(url) .. " 2>&1"
-    local pipe = HostShell.popen(cmd, "rw")
-    if not pipe then return nil, "could not run curl" end
-    local writeOk, werr = pcall(pipe.write, pipe, body)
-    if not writeOk then
-      HostShell.pclose(pipe)
-      return nil, "could not write body: " .. tostring(werr)
+    local pipe = HostShell.popen(cmd)
+    if not pipe then
+      pcall(os.remove, bodyPath)
+      return nil, "could not run curl"
     end
     local readOk, out = pcall(function() return pipe:read("*a") end)
     HostShell.pclose(pipe)
+    pcall(os.remove, bodyPath)
     if not readOk then
       return nil, fetchError(url, nil, tostring(out))
     end

tests/engine/host_shell_postlog.lua

@@ -0,0 +1,75 @@
+-- HostShell.httpPost must work with Lua/LuaJIT's one-way io.popen.
+--
+-- io.popen accepts "r" or "w", not "rw".  POST needs both a request body
+-- and a response status, so the body is staged in a temporary file and curl
+-- is opened read-only for its response.
+--   luajit tests/engine/host_shell_postlog.lua
+
+package.path = "./?.lua;./?/init.lua;" .. package.path
+
+local T = require("tests.harness")
+local check, eq = T.check, T.eq
+local HostShell = require("src.core.HostShell")
+
+local MARK = "\n__gen1recomp_http__"
+local BODY_PATH = "/tmp/gen1recomp-postlog-body-test"
+local URL = "https://logs.example.com/logs"
+local BODY = "debug log body\n"
+
+local realOpen = io.open
+local realPopen = io.popen
+local realTmpname = os.tmpname
+local realRemove = os.remove
+local realHaveCurl = HostShell.haveCurl
+
+local openedPath, openedMode, writtenBody
+local popenCommand, popenMode, removedPath
+
+HostShell.haveCurl = function() return true end
+os.tmpname = function() return BODY_PATH end
+os.remove = function(path)
+  removedPath = path
+  return true
+end
+
+io.open = function(path, mode)
+  openedPath, openedMode = path, mode
+  return {
+    write = function(_, value)
+      writtenBody = value
+      return true
+    end,
+    close = function() return true end,
+  }
+end
+
+io.popen = function(command, mode)
+  popenCommand, popenMode = command, mode
+  return {
+    read = function() return MARK .. "200" end,
+    close = function() return true end,
+  }
+end
+
+local ok, err = HostShell.httpPost(URL, BODY, "text/plain", "gen1recomp-mod/test", 10)
+
+io.open = realOpen
+io.popen = realPopen
+os.tmpname = realTmpname
+os.remove = realRemove
+HostShell.haveCurl = realHaveCurl
+
+eq(ok, true, "a desktop POST succeeds through the read-only response pipe: " .. tostring(err))
+eq(openedPath, BODY_PATH, "the request body is written to a temporary file")
+eq(openedMode, "wb", "the temporary request body is opened for binary writing")
+eq(writtenBody, BODY, "the complete log body is staged")
+eq(popenMode, "r", "curl is opened in the supported read-only mode")
+check(popenCommand:find("--data-binary", 1, true) ~= nil,
+  "curl reads the staged body with --data-binary")
+check(popenCommand:find(BODY_PATH, 1, true) ~= nil,
+  "curl receives the temporary body path")
+check(popenCommand:find(BODY, 1, true) == nil,
+  "the log body is not placed directly in the command line")
+eq(removedPath, BODY_PATH, "the temporary request body is removed")
+
+T.finish("host shell postlog")