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

fix(targets): Validate connectUrl before submission #358

Merged
merged 2 commits into from
Jan 5, 2022

Conversation

jan-law
Copy link
Contributor

@jan-law jan-law commented Jan 5, 2022

Fixes #348

The create button is disabled until a user enters text into the Connect URL field. If the submission is non-empty and invalid, clicking Create will display more helper text instead of posting a request.

@jan-law jan-law added the fix label Jan 5, 2022
@jan-law jan-law requested a review from andrewazores January 5, 2022 18:18
Comment on lines 53 to 57
const jmxServiceUrlFormat = new RegExp([
/service:jmx:([a-z]+):/
,/\/\/([a-z0-9-]+)\/jndi\/([a-z]+):/
,/\/\/([a-z0-9-]+):([0-9]+)/
,/\/([a-z]+)/g
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When validating JMX service URLs, how specific should the expression matching be? I've allowed any JMX connector and am not sure if we should require jndi.

Copy link
Member

Choose a reason for hiding this comment

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

It should be fairly lenient. We basically just want to prevent the user from trying to add a connection with an empty URL, or a URL that is obviously not a JMX service URL (like one with http:// scheme, or just some garbled text instead of a URL). Different connectors and transports should not be checked here since the frontend doesn't know what the backend actually supports.

IMO the objective for fixing #348 is just to help prevent users from doing something obviously wrong and possibly confusing, like seeing the "Create" button lit up as if it's clickable (implying their form submission is complete and valid), and then immediately receive an error response because they had forgotten to actually enter a URL.

@andrewazores andrewazores merged commit 78916d1 into cryostatio:main Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom targets creation form should not allow submission without an entered URL
2 participants