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.

Sign up for GitHub

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

Jump to bottom

Show images after used url_prefix #1453

Merged
merged 2 commits into from
Feb 5, 2020
Merged

Show images after used url_prefix #1453

merged 2 commits into from
Feb 5, 2020

Conversation

anijra
Copy link
Contributor

@anijra anijra commented Dec 30, 2019

After using AWS S3 as the source of external images, they do not appear in the rich text editor and in the modal of inserting images, this pull request will be resolved this problem.

@andrerom andrerom requested review from emodric and glye January 30, 2020 11:20
andrerom
andrerom approved these changes Jan 30, 2020
View reviewed changes
Copy link
Contributor

@andrerom andrerom left a comment

Choose a reason for hiding this comment

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

Looks somewhat correct, but calling on some more reviewers to have a look.

glye
glye approved these changes Jan 30, 2020
View reviewed changes
Copy link
Member

@glye glye left a comment

Choose a reason for hiding this comment

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

Ditto. Looks OK, but this code is hard to follow. Will need more review and careful testing.
So before this fix, the external images gets the same hostname as the site? While after the fix you get the external AWS hostname (and path)?

@anijra
Copy link
Contributor Author

anijra commented Feb 4, 2020

Hi, before the patch the external images do not receive the same host name just "/", and by the patch it receives the AWS host.
For the path, in both they are correct.

@glye
Copy link
Member

glye commented Feb 4, 2020

@anijra Thanks. I'm wondering if image links on non-AWS sites also will include the hostname with this fix. It should work well in either case.

@andrerom andrerom merged commit 2f7d739 into ezsystems:2019.03 Feb 5, 2020
Opencontent pushed a commit to Opencontent/ezpublish-legacy that referenced this pull request Mar 17, 2020
@lrealdi
Merge commit 'c49eef93f50c665a3d7a6b2aa6387ccd46e059f5' into temp
69230ac 
* commit 'c49eef93f50c665a3d7a6b2aa6387ccd46e059f5':
  Removed invalid constructor call
  Fixed wrong old-style constructor usage
  EZP-31040: Remote Code Execution in file uploads
  Show images after used url_prefix (ezsystems#1453)
  improve php 7 bc doc (ezsystems#1452)
  Fix notice when checking for anonymous  classes in autoload generator (ezsystems#1450)
  Do not support literal HTML in the Administration Interface (ezsystems#1408)
  Fix transformURI() on ignoreIndexDir & ! htmlEscape (ezsystems#1449)
  Make autoloads ignore anonymous classes (ezsystems#1448)
  Update php7.md
  Fixing search in media lib which used to loose context (ezsystems#1433)
  [Travis] Add testing for PHP 7.2 and 7.3 (ezsystems#1446)
  EZP-30834: remove strtotime function from the trashed-days option (ezsystems#1441)
  Fix instances of count() that would cause a warning in 7.2
  Updated dbupdate scripts so they will no longer fail when there are multiple users with the same e-mail (ezsystems#1445)
pkamps pushed a commit to mugoweb/ezpublish-legacy that referenced this pull request Apr 3, 2020
@anijra @pkamps
Show images after used url_prefix (ezsystems#1453)
fbc08c4 
* Show images after used url_prefix
* CS
@pkamps pkamps mentioned this pull request Apr 3, 2020
Merged
pkamps added a commit to mugoweb/ezpublish-legacy that referenced this pull request Apr 3, 2020
@pkamps @anijra
Show images after used url_prefix (ezsystems#1453) (#179)
b5d60f3 
* Show images after used url_prefix
* CS

Co-authored-by: Anis Jrad <contact@anisjrad.com>
@emodric
Copy link
Collaborator

emodric commented Jan 22, 2021

Hi!

It seems this issue causes a regression where image added to a XML text field ends up with a starting double slash in the source code:

image

This ultimately causes Chrome to request the image with http://var/... URI, which causes the image not to be displayed in the editor.

/cc @iherak

@glye
Copy link
Member

glye commented Jan 22, 2021

@emodric This is in cases not using AWS S3, or what?

@emodric
Copy link
Collaborator

emodric commented Jan 22, 2021

Yep, normal local install, no DFS or nothing.

@glye
Copy link
Member

glye commented Jan 22, 2021

So the quickest solution is to revert this. Better to break AWS, than everything else (at least non-dfs). And then do a better-tested aws fix later. 2nd opinion... @bdunogier maybe?

@emodric emodric mentioned this pull request Jan 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glye glye glye approved these changes

@andrerom andrerom andrerom approved these changes

@bdunogier bdunogier Awaiting requested review from bdunogier

@emodric emodric Awaiting requested review from emodric

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anijra @glye @emodric @andrerom