- @phenaproxima opened merge request.
- Issue created by @phenaproxima
Automatically closed - issue fixed for 2 weeks with no activity.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Asked @larowlan to double-check my security concerns, and per https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... seems like he's +1 ๐
Automatically closed - issue fixed for 2 weeks with no activity.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Finished reviewing the MR. Epic work! ๐คฉ Confirmed that it meets all 5 goals outlined in the issue summary! ๐
My commits/self-addressed:
- Comment nits.
- A fix for using root-relative URLs for the example images rather than absolute ones
Remaining feedback:
- Spotted a few bits of missing test coverage.
- Everywhere this MR uses "default", it should use "example", because of ๐ DX & authoring experience: for SDC+code components, the example is treated as the default, may not be desirable Active . (Yes, I know this is how we've been talking about it for a while, but that doesn't mean we shouldn't realize our mistake and get better. Not your fault, but this MR understandably introduces a lot more uses of this terminology! So: time to rectify it now.)
- Concern: manually creating a
File
entity in a test โ what am I missing? - Major concern: the way this paves the path for โจ [PP-1] Add support for example images for Code Components' image props Postponed is nice but AFAICT insecure ๐ โ I think it actually might be better to REMOVE this bit from the MR, because of the security aspects, but also because it's not in scope for this issue (for that very reason).
- In fact, a similar concern exists when just using
encoded_files
itself; a config export+import could become an attack vector. See https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... + https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... + https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
Thanks, @larowlan for creating ๐ [PP-1] Consider adding ramsey/uuid for hash-based UUID (v5) generation for default files Postponed: needs info โ linked to it ๐
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
Thanks for the excellent docs addition, that made it 10x easier to review this! Still in the process of doing so.
Created โจ [PP-1] Add support for example images for Code Components' image props Postponed for the actual feature that this is intended to unblock.