CC: Tweaked

CC: Tweaked

42M Downloads

Multiple `http.request`s to the same url cannot be differentiated

fatboychummy opened this issue · 6 comments

commented

When making multiple requests using http.request to the same url, the response given in the http_success / http_failure events cannot be differentiated, causing some problems.

As an example, using my KristWrap (Just for the toKristWalletFormat method and the json.encode) along with its recommended json/sha libraries, I can send two POST requests to krist's 'make a transaction' endpoint. If one of those fails, which failed? I cannot know.

Using the following code which sends two async requests and then listens for responses, we can show this problem in action:

local uri = "https://krist.ceriat.net/transactions/"
local kw = require "KristWrap"
local json = require "json"
local pkey = kw.toKristWalletFormat(pkey) -- pkey removed for obvious reasons

local headers = {["content-type"] = "Application/json"}

local fail = json.encode {
  privatekey = pkey,
  to = "kbielbeajd",
  amount = "fdsklf",
  metadata = "Test"
}

local pass = json.encode {
  privatekey = pkey,
  to = "kbielbeajd",
  amount = 1,
  metadata = "Test"
}

-- write the data to a file rather than printing it out
local function writeFile(data)
  local i = 0
  local format = "output%d"
  repeat i = i + 1 until not fs.exists(string.format(format, i))

  local h = fs.open(string.format(format, i), 'w')
  h.write(data)
  h.close()
end

-- listen for http_success / http_failure events
local function Read()
  local count = 0
  while true do
    local event, uuu, h = os.pullEvent()
    if event == "http_success" then
      local data = h.readAll()
      local code = h.getResponseCode()

      -- combine data, write it to file
      local combined = string.format("RESPONSE CODE: %d\n\n%s", code, data)
      writeFile(combined)
      print("Receive", i)
      count = count + 1
    elseif event == "http_failure" then
      print("Fail response", i)
      writeFile("Failure to send.")
      count = count + 1
    end

    if count >= 2 then break end
  end
end

-- post the transaction requests
local function Post()
  local failed = false
  if math.random(0, 1) == 0 then -- 50/50 chance that we will fail first time or second time.
    http.request(uri, pass, headers)
  else
    http.request(uri, fail, headers)
    failed = true
  end
  print("Send 1")

  if failed then
    http.request(uri, pass, headers)
  else
    http.request(uri, fail, headers)
  end
  print("Send 2")
end

parallel.waitForAll(Read, Post)

And responses, output1:

RESPONSE CODE: 200

{"ok":false,"error":"invalid_parameter","parameter":"amount"}

output2:

RESPONSE CODE: 200

{"ok":true,"transaction":{"id":1910640,"from":"kpcispvv6p","to":"kbielbeajd","value":1,"time":"2020-12-26T22:46:30.611Z","metadata":"Test","type":"transfer"}}

We can see one of them failed, but which one? We can assume the first response coordinates with the first request, but what if timing things happened and the first response was actually to the second request?

I recommend adding a numerical id, returned by http.request that we can use in a comparison from http_success to determine the specific request id something is responding to, for example:

local idreq = http.request("http://whatever.whatever")
local event, url, handle, idresp = os.pullEvent("http_success")
if idreq == idresp then
  -- yay we've got the right one
end
commented

I recommend adding a numerical id, returned by http.request that we can use in a comparison from http_success to determine the specific request id something is responding to, for example:

Yeah. I've been discussing this for years, but really not a fan of it. It probably is the only feasible solution, but chucking more things in the event feels a little ugly too.

commented

Wouldn’t replacing the URL and adding a numerical ID keep the same number of event arguments?

commented

Wouldn’t replacing the URL and adding a numerical ID keep the same number of event arguments?

Yes, but it would break every program which uses http.request directly. :/

commented

Unless there's a use case for the URL that's returned by the event outside of comparing it with the initial requested URL, I don't think that every program that uses it directly would break, would it?

commented

For what it's worth, the way we usually solve it (and have in other Krist implementations) is by using URL fragments to attach an incrementing/random identifier to the end of the URL, like:

local url = "https://krist.ceriat.net/addresses#123"
http.request(url)

local event, u = os.pullEvent("http_success")
if u == url then
  -- This is the correct request
end

Of course, in a serious implementation you'd probably use a table of URLs mapped to response handlers or something.

PS: this trick works for websockets too

commented

I don't think that every program that uses it directly would break, would it?

Unfortunately a great many would, mine included. I usually "build" my urls and then compare the result to what was returned (allows for quickly changing what url we use by only changing one thing), so like:

local urlFormat = "https://%s/%s"
local website = "krist.ceriat.net"
local endpoint = "transactions/new"
local builtUrl = string.format(urlFormat, website, endpoint)

http.request(builtUrl, --[[...]])

local event, url, body = os.pullevent("http_success")
if url == builtUrl then
  --[[...]]

If url was an id rather than the url, my code which uses http.request would break.