Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple http.requests to the same url cannot be differentiated #629

Open
fatboychummy opened this issue Dec 26, 2020 · 6 comments
Open
Labels
area-Core This affects CC's core (the Lua runtime, APIs, computer internals). enhancement An extension of a feature or a new feature.

Comments

@fatboychummy
Copy link
Contributor

fatboychummy commented Dec 26, 2020

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
@fatboychummy fatboychummy added the enhancement An extension of a feature or a new feature. label Dec 26, 2020
@Lemmmy
Copy link
Contributor

Lemmmy commented Dec 26, 2020

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

@SquidDev SquidDev added the area-Core This affects CC's core (the Lua runtime, APIs, computer internals). label Dec 27, 2020
@SquidDev
Copy link
Member

SquidDev commented Dec 27, 2020

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.

@hugeblank
Copy link
Member

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

@SquidDev
Copy link
Member

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. :/

@hugeblank
Copy link
Member

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?

@fatboychummy
Copy link
Contributor Author

fatboychummy commented Dec 27, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Core This affects CC's core (the Lua runtime, APIs, computer internals). enhancement An extension of a feature or a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants