Skip to content

Fix displayImage command and validation#2040

Open
matyson wants to merge 1 commit intomxcube:developfrom
matyson:fix-displayimg
Open

Fix displayImage command and validation#2040
matyson wants to merge 1 commit intomxcube:developfrom
matyson:fix-displayimg

Conversation

@matyson
Copy link
Contributor

@matyson matyson commented Mar 5, 2026

More on displayImage executeCommand. I've noticed that the adapter display_image is expecting the param img_num instead of imgNum and that it did'nt had a type annotation so it was never getting validated. Also, for the path arg the validate_input_str does not handle the case if the string is a valid path string. So, for the changes:

  • make input string validation include path chars ( "/", "-") but this IMO should be handled in a dedicated validator for path strings, which i don't know if it is in development along with the pydantic schemas that could be great addition to the resource handler validation. Please, share thoughts on this one.
  • camelCase to snake_case object call in reducer: i wonder if this one could be automatically converted by the resource_handler ??
  • include type hint in img_num param to get it validated by resource handler

- make input string validation include path chars
- camelCase to snake_case object call in reducer
- include type hint in img_num param to get it validated by resource handler
@axelboc
Copy link
Collaborator

axelboc commented Mar 5, 2026

camelCase to snake_case object call in reducer: i wonder if this one could be automatically converted by the resource_handler ??

Just my two cents on this: I think it's better to be explicit as it helps when searching through the codebase for instance. I've had experiences with frameworks that do this sort of casing conversions in the past and it's always been a nightmare.

Hopefully one day we can generate TypeScript types from the Pydantic models to avoid this sort of issues.

@matyson
Copy link
Contributor Author

matyson commented Mar 5, 2026

Hopefully one day we can generate TypeScript types from the Pydantic models to avoid this sort of issues.

Oh yeah, that would be the dream! ( life would already be better just having TS 😭)

bool: True if the string is valid, False otherwise.
"""
pattern = r"^[a-zA-Z0-9._]*$"
pattern = r"^[a-zA-Z0-9._/-]*$"
Copy link
Member

@marcus-oscarsson marcus-oscarsson Mar 5, 2026

Choose a reason for hiding this comment

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

I have a doubt about /, where do you need that, I imagine for a path ?

We should perhaps have a specific validation made for paths, to avoid i.e ../../ and other sort of possibly "nasty" things ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The display_image(self, path: str, img_num: int) function expects a path to the images masterfile and the validator blocks it. I also think that it should be a different type of validation for this specific type, how do you propose to include this custom type?

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.

3 participants