server icon indicating copy to clipboard operation
server copied to clipboard

[Bug/Feature Request/] Strip `priority` of quotes/apostrophes to prevent marshal cast error

Open OdinVex opened this issue 4 months ago • 11 comments

Is your feature request related to a problem? Please describe. Several backends I use for API automation (such as Huginn) try passing every JSON parameter and value enclosed in quotes, such as "priority": "10". This causes Gotify to return a 400 because "10" is being passed to the API instead of 10, in this example.

Describe the solution you'd like Allow for some flexibility in the parameter for parsing as string->int or int by removing quotes/apostrophes so either becomes capable.

Describe alternatives you've considered Rewriting various backends/other software, but that's insurmountable.

Additional context message API, title, priority, message are all set. Ex:

{
  "title": "...",
  "priority": "10",
  "message": "...",
  "body": "{\"error\":\"Bad Request\",\"errorCode\":400,\"errorDescription\":\"json: cannot unmarshal string into Go struct field MessageExternal.priority of type int\"}",
  "status": 400,
  "headers": {
    ... Removed for Brevity ...
  }
}

OdinVex avatar Nov 02 '25 00:11 OdinVex

I'm unsure about this, I like endpoints being strict as this makes the implementation more easy and forces you to use it correctly.

I'd probably be okay with making an exception for the priority field and allowing string numbers there when there are more tools that force number strings in json. If it's just one, then I don't think I want this included.

jmattheis avatar Nov 03 '25 17:11 jmattheis

I'd normally be entirely in agreement about API endpoints, but I think integer/float parsing from string to int/float is rather common among API handling, in general, because it's rather common and safe. I'll try to raise the issue with Huginn and the other backends for now, thanks for following up.

OdinVex avatar Nov 03 '25 17:11 OdinVex

referencing the upstream issue https://github.com/huginn/huginn/issues/2661 to help other users find the suggested fix (even though people reported it didnt but linking helps people find it faster 😅 )

LaurenceJJones avatar Nov 05 '25 16:11 LaurenceJJones

referencing the upstream issue huginn/huginn#2661 to help other users find the suggested fix (even though people reported it didnt but linking helps people find it faster 😅 )

I forgot to do that, thank you very much. 👍

OdinVex avatar Nov 05 '25 16:11 OdinVex

Thanks for the context. I think one way we can potentially do this without hacks is allow priority to be specified on the URL even if the payload is submitted in JSON, I assume you could do something like:

{
  "post_url": "https://my.gotify.box/message?priority={{priority}}",
   // the other fields
}

This could be useful for other generic Webhook implementations as well where you just want to hardcode a priority without the app explicitly supporting it.

eternal-flame-AD avatar Nov 05 '25 16:11 eternal-flame-AD

Thanks for the context. I think one way we can potentially do this without hacks is allow priority to be specified on the URL even if the payload is submitted in JSON, I assume you could do something like:

{ "post_url": "https://my.gotify.box/message?priority={{priority}}", // the other fields }

This could be useful for other generic Webhook implementations as well where you just want to hardcode a priority without the app explicitly supporting it.

GET vs POST, in short? Might it be a bit cumbersome compared to a simpler parse for a rather unique field?

OdinVex avatar Nov 05 '25 16:11 OdinVex

Hmmm, not necessarily GET vs. POST. The current way of handling payload is rather binary:

You either put everything all on the URL and use GET-like semantics , like /message?token=<>&priority=<>&messsage=<> (which is lax in typing but suboptimal), or you have to put everything in JSON and use POST (which makes you subject to strict type checking).

I think the issue being debated here is when a fully fledged JSON templating engine is just not there, who is responsible for bending backwards for the other's constraints?

What I think would be a compromise without making the API type lax is you just send your regular payload in JSON but you could put a default priority in URL to override the "default , default" priority (which is 0).

eternal-flame-AD avatar Nov 05 '25 16:11 eternal-flame-AD

Actually to be more clear in my wording, it should be an "inheritance" of priority , so the default is zero, then the default priority of the application is used, and then (I propose) the ?priority= query parameter be used, and then whatever is in the actual JSON payload priority is used.

eternal-flame-AD avatar Nov 05 '25 16:11 eternal-flame-AD

Sounds like simple casting would be an easier solution that handles everything.

OdinVex avatar Nov 05 '25 16:11 OdinVex

I prefer a solution that adheres to design principles (JSON is type strict, query parameters are not), is more flexible and future proof, not necessarily the "simplest" amount of change just to make this ticket disappear.

I am not sure what you meant by "handles everything" .. it's more like handles everything (that has this exact limitation, which so far only one app is brought up. Jannis' original stance "If it's just one, then I don't think I want this included." feels proportional to me.

A concrete worked example would be let's say you have a weather checking agent, and you want it to send you a notification of 9 if there is a tornado warning, even if we implemented your suggestion this still would not be workable, but with a proper inheritance you could program the agent to do:

IF (report.type = TORNADO)
   SEND WEBHOOK /message?priority=9
ELSE
  SEND WEBHOOK /message
FI

eternal-flame-AD avatar Nov 05 '25 17:11 eternal-flame-AD

I prefer a solution that adheres to design principles (JSON is type strict, query parameters are not), is more flexible and future proof, not necessarily the "simplest" amount of change just to make this ticket disappear.

I am not sure what you meant by "handles everything" .. it's more like handles everything (that has this exact limitation, which so far only one app is brought up. Jannis' original stance "If it's just one, then I don't think I want this included." feels proportional to me.

A concrete worked example would be let's say you have a weather checking agent, and you want it to send you a notification of 9 if there is a tornado warning, even if we implemented your suggestion this still would not be workable, but with a proper inheritance you could program the agent to do:

IF (report.type = TORNADO)
   SEND WEBHOOK /message?priority=9
ELSE
  SEND WEBHOOK /message
FI

I think I was trying to point out it seems to me going for parameters is just another long-way of casting. The only issue I've seen in parsing is string to int, so just trying to cast from string to int and then parsing as int solves anyone's issue with priority. I mentioned there are other backends that have this issue, it's not just one "app", but a simple attempt at trying to cast and then parsing as int would resolve any form of it. "even if we implemented your suggestion this still would not be workable" ??? "9" will parse. If you want some kind of inheritance thing (separate from this Issue) then you could file that under a new Issue to give it the proper attention it'd deserves. This Issue is about parsing strings to int/handling priority.

OdinVex avatar Nov 05 '25 17:11 OdinVex