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

add nic option config for json response #17

Merged
merged 2 commits into from
Jun 6, 2021
Merged

add nic option config for json response #17

merged 2 commits into from
Jun 6, 2021

Conversation

BuckarooBanzay
Copy link
Member

@BuckarooBanzay BuckarooBanzay commented Mar 4, 2021

This PR adds the ability to pass a config object to the nic as well as json parsing and header settings.
The message can still be a plain url as text or an object in the form:

{
 url = "http://x.y",
 parse_json = true
)

I've tested it and it works well but there may be some crash vectors, not sure 🤷

Example

Luacontroller

if event.type == "program" then
 digiline_send("nic", {
  url = "http://api.icndb.com/jokes/random",
  parse_json = true
 })
end

if event.type == "digiline" then
 print(event.msg)
end

Console

{
	completed = true,
	timeout = false,
	succeeded = true,
	data = {
		value = {
			id = 34,
			categories = {
				
			},
			joke = "The opening scene of the movie "Saving Private Ryan" is loosely based on games of dodgeball Chuck Norris played in second grade."
		},
		type = "success"
	},
	code = 200
}

@BuckarooBanzay BuckarooBanzay added the enhancement New feature or request label Mar 4, 2021
nic.lua Outdated
elseif type(msg) == "table" and type(msg.url) == "string" then
-- config object
url = msg.url
headers = msg.headers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should check type of headers before using it.

Copy link
Member

@S-S-X S-S-X Mar 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also contents, if you send headers = {false} it will crash server.
Probably something like:

if type(msg.headers) == "table" and #msg.headers > 0 and #msg.headers < 50 then
  headers = {}
  for _, header in ipairs(msg.headers) do
    header = (tostring(header)):gmatch("%C+")()
    if header ~= "" then
      table.insert(headers, header)
    end
  end
end

ipairs is intended over pairs to drop hash keys.

@S-S-X
Copy link
Member

S-S-X commented Mar 8, 2021

Tested above with this crap:

local msg = { headers =
{ 1, 2, 3, false, true, {"woo"}, function()quit()end, "Authorization: Ajadshk:dasjhghja", [[
And what
			about			!!!
this
crap????
]]
}
}
table.insert(msg.headers, msg.headers)
msg.headers.really = msg.headers[#msg.headers]

local headers
if type(msg.headers) == "table" and #msg.headers > 0 and #msg.headers < 50 then
  headers = {}
  for _, header in ipairs(msg.headers) do
    header = (tostring(header)):gmatch("%C+")()
    if header ~= "" then
      table.insert(headers, header)
    end
  end
end

http.fetch({
		url="http://127.0.0.1:10101/?hello",
		timeout=5,
		user_agent="Minetest Digilines Modem",
		extra_headers = headers
	},
	function(r) print(dump(r)) end,
	1
)

Headers after running through filter was:

{
	"1",
	"2",
	"3",
	"false",
	"true",
	"table: 0x555983663440",
	"function: 0x555983663490",
	"Authorization: Ajadshk:dasjhghja",
	"And what",
	"table: 0x5559836633f0"
}

@S-S-X
Copy link
Member

S-S-X commented Mar 8, 2021

Also for complete test, request I get for that is:

GET /?hello HTTP/1.1
Host: 127.0.0.1:10101
User-Agent: Minetest Digilines Modem
Accept: */*
Accept-Encoding: gzip
table: 0x555983663440
function: 0x555983663490
Authorization: Ajadshk:dasjhghja
table: 0x5559836633f0

@BuckarooBanzay
Copy link
Member Author

i've removed the headers option completely, not worth the trouble IMO.

Also: some fixed headers could be set in the future, for example if POST would be implemented with a json-object, the headers could be just `Content-Type: application/json``

@BuckarooBanzay BuckarooBanzay changed the title add nic option config for json response and headers add nic option config for json response Jun 1, 2021
@OgelGames
Copy link
Contributor

OgelGames commented Jun 1, 2021

luacontroller_json.lua 👀

Though really the ideal solution would be to include minetest.parse_json and minetest.write_json in the luacontroller sandbox, instead of having it only be for the nic.

There is a PR open to add them, along with a heap of other helper functions: minetest-mods/mesecons#507, though this other one looks like it's more likely to get merged: minetest-mods/mesecons#557, and would allow for even more functionality.

EDIT: then again, there is the huge benefit of the parsing being done outside of the luacontroller sandbox, so still 👍 for this PR :)

@OgelGames
Copy link
Contributor

I don't see any objections to this, so merging it.

@OgelGames OgelGames merged commit 126aa82 into master Jun 6, 2021
@OgelGames OgelGames deleted the nic-json branch June 6, 2021 07:21
@S-S-X S-S-X mentioned this pull request Sep 28, 2021
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants