feat: allow mermaid to be configured via some query params

[SayakMukhopadhyay]

This PR tries to implement the ability of configuring some options of mermaid using query params of the GET request.

This will only handle properties that have string values and won't handle anything else at the moment. This is mostly because I am not sure how to send configuration values of flowchart.useMaxWidth. If there is some clarity on that I can implement that too.

Fixes #1326

[ggrossetie]

This will only handle properties that have string values and won't handle anything else at the moment. This is mostly because I am not sure how to send configuration values of flowchart.useMaxWidth. If there is some clarity on that I can implement that too.

I think we need to establish a convention for configuration using nested names. We should also decide how we can infer the value type (array, boolean, number...).

Related: #1410 (comment)

Configuration can be sent ad HTTP headers. In theory, we can use . in the header name:

tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA
token = 1*tchar
header-field   = field-name ":" OWS field-value OWS

Reference: https://www.rfc-editor.org/rfc/rfc7230#section-3.2

But some providers, for instance Cloudflare, apply restrictions:

The name of the HTTP request header you want to set or remove can only contain:

• Alphanumeric characters: a-z, A-Z, and 0-9
• The following special characters: - and _

Reference: https://developers.cloudflare.com/rules/transform/request-header-modification/reference/header-format/

So, it might be safer to use a mix of snake_case and kebab-case. For instance:

flowchart.useMaxWidth -> Kroki-Diagram-Options-Flowchart_Use-Max-Width

Please note that header name are case insensitive.
If we stick to the RFC, then I think we should use . and -:

flowchart.useMaxWidth -> Kroki-Diagram-Options-Flowchart.Use-Max-Width

As mentioned in #1410 I'm leaning toward HTTP headers (rather than query parameters) to send diagram options from the gateway server to a companion container. I think it has less restrictions than a query parameters (especially regarding the URL length).

[SayakMukhopadhyay]

yeah, Even I was considering headers since the size of the Get path itself could be large enough (not any longer. see my comment below). As for the Header convention, it would better to use the Cloudflare sub conventions since its extremely popular (and absolutely not because I use it😛).

But the case insensitiveness of Headers can be problematic. We will need to keep track of all mermaid options and make case insensitive matches to figure out which property is which. This might become more problematic if mermaid expects a case sensitive value.

Any opinions on this situation?

[ggrossetie]

But the case insensitiveness of Headers can be problematic. We will need to keep track of all mermaid options and make case insensitive matches to figure out which property is which. This might become more problematic if mermaid expects a case sensitive value.
Any opinions on this situation?

I don't think that's required, we can use the following transformations:

1. to lowercase on the header name: Kroki-Diagram-Options-Flowchart_Use-Max-Width -> kroki-diagram-options-flowchart_use-max-width
2. remove the prefix part: kroki-diagram-options-flowchart_use-max-width -> flowchart_use-max-width
3. replace _ by .: flowchart_use-max-width -> flowchart.use-max-width
4. capitalize after - and remove -: flowchart.use-max-width -> flowchart.useMaxWidth

[SayakMukhopadhyay]

Sounds good, let me see how quickly I can cook something up.

[SayakMukhopadhyay]

Wait, I don't think this will work with how things are now. RIght now, I am dependendant on the mkdocs kroki plugin (https://github.com/AVATEAM-IT-SYSTEMHAUS/mkdocs-kroki-plugin) cause my only usecase is with mkdocs at this point. And although that plugin provides a GET and POST methods, they provide no way to send any info via headers.

Moreover, imagine that plugin could do that, it would still not solve the problem of generating a diagram with a theme given at runtime. Right now, the diagram is fetched via a img tag's src attribute. How is this going to be changed to accommodate headers.

I didn't think of this issue before and was thinking about headers being a good idea but at this point I don't see a way to pass parameters to mermaid without using query params or changing a lot of tools around this ecosystem.

[SayakMukhopadhyay]

As an alternative, we could pass the data, but using query parameters instead. So for flowchart max width we send ?flowchart.useMaxWidth=true. This should make the URL usable in an ` tag. Using, headers or POST body would simply restrict when the mermaid options can be used.

[ggrossetie]

Wait, I don't think this will work with how things are now. RIght now, I am dependendant on the mkdocs kroki plugin (AVATEAM-IT-SYSTEMHAUS/mkdocs-kroki-plugin) cause my only usecase is with mkdocs at this point. And although that plugin provides a GET and POST methods, they provide no way to send any info via headers.

In AsciiDoc, we are using block attributes to send diagram options.

Moreover, imagine that plugin could do that, it would still not solve the problem of generating a diagram with a theme given at runtime. Right now, the diagram is fetched via a img tag's src attribute. How is this going to be changed to accommodate headers.

Sorry I wasn't clear.
The gateway server (entry-point of the API) will still support diagram options sent as query parameters, as attributes in the JSON body or as HTTP headers.
I want to use HTTP headers (instead of query parameters) for internal communications between the gateway server and the companion containers.

I didn't think of this issue before and was thinking about headers being a good idea but at this point I don't see a way to pass parameters to mermaid without using query params or changing a lot of tools around this ecosystem.

You don't, the gateway server will receive diagram options as query parameters, as attributes in the JSON body or as HTTP headers and pass them as HTTP headers to the Mermaid companion container.

[ggrossetie]

To give more context, I want to document/specify the HTTP endpoint of the companion containers so others can contribute additional companion containers.
Let's say that you want to use diagram library "foo" (which isn't included in Kroki by default). In that case, you would be able to create an HTTP service that implements the "companion container specification". The gateway server will "automatically" discover this new service and you will be able to convert "foo" diagrams using:

GET http://my.kroki/foo/svg/diagram?some-option=bar

[SayakMukhopadhyay]

Let me see if I understand this correctly. The Java based Gatewat Server (which has the Mermaid.js service) already implement the handling of query parameters, body or headers and handles them by stripping all query parameters if present, body if present and makes a request with only Headers to the Js + micro based mermaid server. Only the mermaid server doesn't handle headers yet.

So, I won't need to make any updates to the gateway server for instance?

[SayakMukhopadhyay]

Just checked, seems like the gateway server is not modifying the headers at all right now. But I get your train of thought. This is going to be a bit of a change but I will see what I can do.

[SayakMukhopadhyay]

Something I noticed, out of all the diagram types that options can be passed to as in https://docs.kroki.io/kroki/setup/diagram-options/, only blockdiag is not implemented in the java server and is implemented as a python server. So, I checked how the gateway was sending options to blockdiag and it turns out that its sending via query params

kroki/blockdiag/src/server.py

Line 117 in 7347d8b

return _generate_diagram(RackdiagApp(), 'block', output_format, source or request.get_data(as_text=True), request.args)

Are you still sure that the gateway should be passing headers to underlying mermaid container? Cause gateway is not passing headers (at least that's what it seems to me) blockdiag.

Moreover, the convert function of blockdiag is identical to how mermaid's is now.

[ggrossetie]

So, I won't need to make any updates to the gateway server for instance?

You're correct, currently, the gateway server makes a request to the Mermaid companion service/container using query parameters:

kroki/server/src/main/java/io/kroki/server/action/Delegator.java

Line 36 in 7347d8b

request.addQueryParam(key, value);

So we will need to change the code to make a request with HTTP headers... but I can take care of that part 😉

Are you still sure that the gateway should be passing headers to underlying mermaid container? Cause gateway is not passing headers (at least that's what it seems to me) blockdiag.

I'm fine with implementing this feature using query parameters and do the change I've mentioned in another pull request. My point was that we need to make sure that we won't have any issue when migrating from query parameters to HTTP headers (more specifically on the naming convention we've established).

[SayakMukhopadhyay]

My point was that we need to make sure that we won't have any issue when migrating from query parameters to HTTP headers (more specifically on the naming convention we've established).

Yeah, and if we are doing it, we should implement something similar to blockdiag too.

So, I will plan for working on this tomorrow. Will this PR be fine or now so that you merge this and we can make the move to Headers in a new PR, or should I make the Header implementations in this PR itself. Note that header implementation means changing the gateway server too. Honestly I can do that too. Its just that I need to know the direction I should follow.

[ggrossetie]

Will this PR be fine or now so that you merge this and we can make the move to Headers in a new PR, or should I make the Header implementations in this PR itself.

I would prefer to migrate from query parameters to HTTP headers in a new pull request.
The remaining items are:

• transform the option name (feat: allow mermaid to be configured via some query params #1413 (comment))
• create an array from a comma separated list for options that expect an array (such as actorColours: https://github.com/mermaid-js/mermaid/blob/cdba2b4ac81947f5751de10311de2dfbe52c738e/packages/mermaid/src/defaultConfig.ts#L859-L862)
• we should also make sure that it won't be possible to change secure, securityLevel, startOnLoad and maxTextSize options: https://github.com/mermaid-js/mermaid/blob/cdba2b4ac81947f5751de10311de2dfbe52c738e/packages/mermaid/src/defaultConfig.ts#L123-L133C3

[SayakMukhopadhyay]

Pushed a new commit to the PR. This commit would handle boolean, number and array values. But this commit will still not work correctly because the gateway is lowercasing all query parameters before forwarding them.

From what I see the gateway server is lowercasing all query parameters in DiagramHandler. From what I know, URLs are case sensitive, at least in the path and query params. Lowercasing headers I understand, but lower casing query params means that I would need to send er.use_max_width in query params too. That mean losing 1 character that could have been data of the 2048 characters the URL can be. Moreover, Delegator.delegate() is using the options (that has both header based and query parameter based configs) and putting them as query parameters for the next container to use.

The way I prefer things to work in the long run:

Options can be passed via query parameters and headers to the gateway server. The gateway server converts the both of them into headers only.

Query params can be passed as property names with dot notation and case preserved. For eg

https://kroki.io/mermaid/svg/eNpLL0osyFDwCeJSAALH6IDEotS8EgXDWAVdXTsFp2jnjMycFCAXIg0WdIYKGiELukAFjRWgJkBlXcCyrlBZE2RBN6igKUgQADblIZ4?theme=dark&er.useMaxWidth=true

Arrays can be passed by using array notation. For eg.

https://kroki.io/mermaid/svg/eNpLL0osyFDwCeJSAALH6IDEotS8EgXDWAVdXTsFp2jnjMycFCAXIg0WdIYKGiELukAFjRWgJkBlXcCyrlBZE2RBN6igKUgQADblIZ4?er.foo=[bar,tar,jar]

The above 2 can be passed as headers in a case insensitive way as such

'kroki-diagram-options-theme': 'dark',
'kroki-diagram-options-er_use-max-width': 'true',
'kroki-diagram-options-er_foo': '[bar,tar,jar]'

The PR in its current state cannot handle query parameters are above. The gateway server converts er.useMaxWidth to er.usemaxwidth. As such the only way I can make the current PR work is if I start sending the query parameters as er.use_max_width. Then I would need to reconvert the underscores in camel case in the mermaid container. But as I mentioned before I would prefer to be able to send correct casing in the URL itself.

Let me know what you think.

[ggrossetie]

I guess we can support both. I like the naming consistency (i.e. we can use the same diagram option name no matter how we send it). If we replace `_` by `.` and dash-separated `-` words by camelCase we can safely send: `er.useMaxWidth=true` or `er_use-MAX-Width=true` (which will be transformed to `er.useMaxWidth=true`). If the name contains `_` or `-` then we use lowercase and apply transformations. Otherwise, we use the name as-is. Le lun. 2 janv. 2023, 11:14, Sayak Mukhopadhyay ***@***.***> a écrit :

…

Pushed a new commit to the PR. This commit would handle boolean, number and array values. But this commit will still not work correctly because the gateway is lowercasing all query parameters before forwarding them. From what I see the gateway server is lowercasing all query parameters in DiagramHandler. From what I know, URLs are case sensitive, at least in the path and query params. Lowercasing headers I understand, but lower casing query params means that I would need to send er.use_max_width in query params too. That mean losing 1 character that could have been data of the 2048 characters the URL can be. Moreover, Delegator.delegate() is using the options (that has both header based and query parameter based configs) and putting them as query parameters for the next container to use. *The way I prefer things to work in the long run:* Options can be passed via query parameters and headers to the gateway server. The gateway server converts the both of them into headers only. Query params can be passed as property names with dot notation and case preserved. For eg https://kroki.io/mermaid/svg/eNpLL0osyFDwCeJSAALH6IDEotS8EgXDWAVdXTsFp2jnjMycFCAXIg0WdIYKGiELukAFjRWgJkBlXcCyrlBZE2RBN6igKUgQADblIZ4?theme=dark&er.useMaxWidth=true Arrays can be passed by using array notation. For eg. https://kroki.io/mermaid/svg/eNpLL0osyFDwCeJSAALH6IDEotS8EgXDWAVdXTsFp2jnjMycFCAXIg0WdIYKGiELukAFjRWgJkBlXcCyrlBZE2RBN6igKUgQADblIZ4?er.foo=[bar,tar,jar] The above 2 can be passed as headers in a case insensitive way as such 'kroki-diagram-options-theme': 'dark', 'kroki-diagram-options-er_use-max-width': 'true', 'kroki-diagram-options-er_foo': '[bar,tar,jar]' The PR in its current state cannot handle query parameters are above. The gateway server converts er.useMaxWidth to er.usemaxwidth. As such the only way I can make the current PR work is if I start sending the query parameters as er.use_max_width. Then I would need to reconvert the underscores in camel case in the mermaid container. But as I mentioned before I would prefer to be able to send correct casing in the URL itself. Let me know what you think. — Reply to this email directly, view it on GitHub <#1413 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACRLXF4OTNJF2TFPWI5QF3WQKS63ANCNFSM6AAAAAATM7RGN4> . You are receiving this because you commented. Message ID: ***@***.***>

[SayakMukhopadhyay]

I have pushed a new commit. This commit handled mermaid configuration properties as such:
As query params

https://kroki.io/mermaid/svg/eNpLy8kvT85ILCpR8AniUlBwjA5ILErNK1EwjFXQ1bVTcIp2zsjMSQFyQZJgIWeokBFCyAUqZKwA1Q2WcwHLuULlTBBCblAh01guANSfIdY=?theme=dark&er_use-max-width=true

Or as Headers

curl --location --request GET 'https://kroki.io/mermaid/svg/eNpLy8kvT85ILCpR8AniUlBwjA5ILErNK1EwjFXQ1bVTcIp2zsjMSQFyQZJgIWeokBFCyAUqZKwA1Q2WcwHLuULlTBBCblAh01guANSfIdY=' \
--header 'kroki-diagram-options-theme: dark' \
--header 'kroki-diagram-options-er_use-max-width: true'

[ggrossetie]

Thanks for you contribution! It looks good 👍🏻
I will try to add a few tests to catch potential regression when we will move from query parameters to HTTP headers.

Anyway, great work 🙌🏻

[ghost]

@SayakMukhopadhyay, that link no longer works?

(Why does a 400 error show OK?)

[SayakMukhopadhyay]

@DaveJarvis The link I entered was an example of how this PR will behave once deployed. I am not sure if this PR has been deployed yet. @Mogztter would be able to tell that.

[ggrossetie]

I am not sure if this PR has been deployed yet. @Mogztter would be able to tell that.

No, I didn't release/deploy a new version yet.

that link no longer works?

Are you suggestion that it was working before?
I don't really know why it returns a 400, query parameters should be ignored.

Why does a 400 error show OK?)

400 could be a syntax error but we don't know precisely. Arguably, we could return something else than OK... I wasn't sure about Bad Request because technically the query is valid even though the diagram syntax is invalid.

[ghost]

technically the query is valid

Would 422 be appropriate?

The 422 (Unprocessable Entity) status code means the server
understands the content type of the request entity (hence a
415(Unsupported Media Type) status code is inappropriate), and the
syntax of the request entity is correct (thus a 400 (Bad Request)
status code is inappropriate) but was unable to process the contained
instructions. For example, this error condition may occur if an XML
request body contains well-formed (i.e., syntactically correct), but
semantically erroneous, XML instructions.

https://www.rfc-editor.org/rfc/rfc4918#section-11.2

Are you suggestion that it was working before?

No, I misunderstood; didn't realize that completing the PR didn't result in deployment.

[SayakMukhopadhyay]

I don't really know why it returns a 400, query parameters should be ignored.

@Mogztter Before my PR, the gateway server passes query parameters in the URL to the mermaid container. But the mermaid container makes a regex check on the URL to see if it ends with "mermaid". Sending query parameters fails this check and hence the error.

[ggrossetie]

Thanks for taking the time to investigate 👌

[imysl0]

So, can I set the gantt.useMaxWidth parameter as shown in the picture now?

[imysl0]

@SayakMukhopadhyay @ggrossetie
The useWidth parameter also does not take effect