Skip to content

Note about Vaadin-Refresh#4254

Merged
TatuLund merged 10 commits into
latestfrom
sudebi-vaadin-refresh
Jun 25, 2025
Merged

Note about Vaadin-Refresh#4254
TatuLund merged 10 commits into
latestfrom
sudebi-vaadin-refresh

Conversation

@mukherjeesudebi
Copy link
Copy Markdown
Contributor

This PR adds a small description about Vaadin-Refresh token

Comment thread articles/flow/advanced/modifying-the-bootstrap-page.adoc Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2025

AI Language Review

In the modified file, there is a new section titled "Forcing a Page Reload After an Invalid Server Response" added under Java Annotations. Overall, the new content integrates well with the existing documentation. However, here are a few suggestions for improvement:

  1. Consistency in Capitalization:

    • Consistency in the capitalization of headings can be improved. The other section titles use backticks for method names. You may consider applying the same formatting to the heading so it is clear and consistent.
  2. Clarification Needed:

    • The sentence "Usually such responses are served by some 3rd party servers and then you need to add the refresh token as a meta tag to the HTML page served by it." might be clearer if rephrased. Consider: "Usually, such responses are served by third-party servers. In this case, you need to add the refresh token as a meta tag to the HTML page returned by the server."
  3. Sentence Structure:

    • The sentence "But sometimes due to proxy or firewall timeout, a new session is served for the client, without the client being aware of that." may be clearer as: "However, sometimes due to proxy or firewall timeouts, a new session is served to the client without the client being aware."

Making these changes would help maintain clarity and consistency throughout the documentation.

@TatuLund TatuLund requested a review from Legioth May 23, 2025 05:24

[source,java]
----
public class AppShell implements AppShellConfigurator {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if this code example makes sense since the unparseable response typically comes from some kind of proxy or authentication filter rather than directly from the regular application.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

True. And it is hard to document, as it depends on the system in question. Just last week I had customer case, where (I assume) reverse proxy shows html page (asking to try again later) when it is congested, and naturally one could try to put Vaadin-Refresh in the meta tag of that html page (probably a static file served).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I have the same issue. Instead of JSON from the Vaadin server, HTML comes from an Microsoft ADFS server, triggered as soon as the authentication token has expired. Now, I added
settings.addMetaTag("refresh", "Vaadin-Refresh");
to my overwridden method
public void configurePage(final AppShellSettings settings)
For whatever reason, I can't see
<meta name="refresh" content="Vaadin-Refresh">
in the HTML of my Vaadin pages. For years, I have
settings.addFavIcon("icon", "/icons/icon.png", "210x210");
settings.addLink("shortcut icon", "/icons/favicon.ico");
in my configurePage() method. These two are in the HTML, but settings.addMetaTag() has no result.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@a1psaitz , " HTML comes from an Microsoft ADFS server" <- You need to add the META tag with "Vaadin-Refresh" string to this file instead of configuring Vaadin's index page.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not able to refactor the response from ADFS. Microsoft ADFS is a SaaS application for us. It's running in the Microsoft cloud.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So, what is the recommended approach here? Though this code snippet is not useful always but in some cases it is.

Should we just mention that the above code would not help if the unparseable response comes from some proxy or 3rd party server and in that case the "Vaadin-Refresh" string needs to be added to the response file instead of configuring Vaadin's index page. But again, it is difficult to configure such files as mostly they are not static and the error can be from any system. So, any suggestions on what generic message would help?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I would add such note. Or put that even first, i.e. typically Vaadin-Refresh token needs to be added to the 3rd party's response page if possible, and the after that describe this special case when it needs to be added to Vaadin's bootstrap.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would suggest : If the response is HTML instead of JSON, the browser should render this HTML.
Why : The HTML can't come from the Vaadin server. There must be a reason for that HTML.
How : Perhaps a new setting in application.yaml is required -> vaalin.renderHtmlOnHeartbeat: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@a1psaitz I have some doubts to what you are proposing, mainly concerning security. My first reaction is, that if it is done that way, it opens an attack vector. @Legioth could comment this as well. I am also thinking that refreshing the page on what ever offending content could be problematic too, however if it is not a default and needs to be enabled by API, it would be then responsibility of you to ensure that in your environment this is safe, or you are willing to carry the potential risks. Idea of the Vaadin-Refresh magic string included in that HTML page is that you had access to do that, and you are this enabling it on this particular page only. Any other tampered or faulty response to the client will still produce an error.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If there's a "man in the middle" that can inject arbitrary content into the response to the server, then you will have a security issue if that party is untrusted regardless of whether Vaadin renders the response as HTML or not (since it could just as well be valid UIDL with instructions to render something as HTML). So from that point of view, I don't think it's a security issue. I still agree that it might be good to have it as an opt-in feature since it might still be surprising.

As for the original discussion about how to document the current feature, one idea could be to just show an example of what the HTML document could look like without going into any details about how that HTML would be generated.

@TatuLund TatuLund merged commit 04813cd into latest Jun 25, 2025
4 of 5 checks passed
@TatuLund TatuLund deleted the sudebi-vaadin-refresh branch June 25, 2025 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants