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

Inserting images via URL does not validate input #7917

Closed
FilipTokarski opened this issue Aug 24, 2020 · 7 comments · Fixed by #8271 · May be fixed by ikomsta/ckeditor5#1
Closed

Inserting images via URL does not validate input #7917

FilipTokarski opened this issue Aug 24, 2020 · 7 comments · Fixed by #8271 · May be fixed by ikomsta/ckeditor5#1
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. intro Good first ticket. package:image squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@FilipTokarski
Copy link
Member

📝 Provide detailed reproduction steps (if any)

  1. On latest release branch go to docs /features/image.html#inserting-images-via-source-url
  2. Try using insert image via URL with following data:
    • pressing space
    • random letters
    • random digits

✔️ Expected result

Input gets validated ( at least rubbish input like the one mentioned above )

❌ Actual result

You can type anything, plugin will insert broken image placeholder or empty widget:

0_img1

📃 Other details

  • Browser: any

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@FilipTokarski FilipTokarski added type:bug This issue reports a buggy (incorrect) behavior. package:image labels Aug 24, 2020
@oleq oleq added type:improvement This issue reports a possible enhancement of an existing feature. domain:ui/ux This issue reports a problem related to UI or UX. squad:core Issue to be handled by the Core team. labels Aug 24, 2020
@oleq
Copy link
Member

oleq commented Aug 24, 2020

I'm glad you brought this up @FilipTokarski, this is a valid improvement. 

I'm not sure if we can validate all URLs because there are millions of use-cases (with protocol, protocol-less, relative, absolute, base64, etc.) but for sure we could filter empty URLs out. Media embed already implements input validation so we could copy the UX.

@mlewand mlewand added this to the backlog milestone Aug 25, 2020
@dzpt
Copy link

dzpt commented Aug 30, 2020

where the hell is /features/image.html? i couldn't find anyway to implement attach image by URL?

@oleq
Copy link
Member

oleq commented Aug 31, 2020

where the hell is /features/image.html? i couldn't find anyway to implement attach image by URL?

Hi @dzpt, it's an unfortunate reference to the project documentation

Anyway, although CKEditor 5 v22.0.0 has already been released on npm, the updated documentation with the feature you're looking for will be out along with the blog post later this week. Until then, check out our nightly docs instead. Sorry for the ambiguity!

@Reinmar
Copy link
Member

Reinmar commented Sep 4, 2020

Solution for now: let's copy the code from the media embed form (it already implements some sort of validation).

What we can do: check for empty values (including trimming). What we shouldn't do: try to be smart on what URL could be or not as even "adfgh" could be a valid relative URL or an image ID in your database.

@Reinmar Reinmar added the intro Good first ticket. label Sep 4, 2020
@Reinmar Reinmar modified the milestones: backlog, nice-to-have Sep 4, 2020
@LangQian
Copy link

Hi, I'm wondering if it is possible to expose the URL validation to be configurable by developers, along with the error messages if something went wrong? Due to the needs of our customers, we need to apply much stricter rules to the inserted hyperlinks or image urls. I'm right now patching the source codes to achieve this, which is not very convenient, since you guys are rolling out new releases every month (which is great, thanks!) and I don't want to check the compatibility of my codes every time I do an update.

I believe different products have different requirements on this depending on the security needs, so if the validation rules can be customizable, that would be really great! Hope you can consider this :)

@pkwasnik
Copy link
Contributor

@olek I see that here when input is empty, the button is disabled, but in media embed not (there is error message). Maybe in media embed I will implement the same, disabling "ok" button when input is empty or has only spaces?

@oleq
Copy link
Member

oleq commented Oct 15, 2020

@olek I see that here when input is empty, the button is disabled, but in media embed not (there is error message). Maybe in media embed I will implement the same, disabling "ok" button when input is empty or has only spaces?

Sounds like a plan!

jodator added a commit that referenced this issue Oct 16, 2020
Fix (image): The insert button in the insert image dropdown will now be disabled when URL input is empty. Closes #7917.

Fix (media-embed): Disable the save button in insert media dropdown when the input is empty. See #7917.
@Reinmar Reinmar modified the milestones: nice-to-have, iteration 37 Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. intro Good first ticket. package:image squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
7 participants