/matrix-react-sdk

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix e2e attachment download by using iframes. #562

Merged
merged 15 commits into from Dec 2, 2016

Conversation

Reviewers

@Kegsay Kegsay

@dbkr dbkr

Assignees

@dbkr dbkr

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@NegativeMjark @matrixbot @dbkr @Kegsay
@NegativeMjark
Contributor

NegativeMjark commented Nov 17, 2016 • edited

Allows users to download attachments that are bigger than 2MB.

NegativeMjark added some commits Nov 16, 2016

@NegativeMjark
Render attachments inside iframes.
01c1203
@NegativeMjark
Merge remote-tracking branch 'origin/develop' into markjh/attachment_… 
…download

Conflicts:
	src/components/views/messages/MFileBody.js
2787845
@NegativeMjark
Fix up the image and video views
30ac145
@NegativeMjark
Fix m.audio

Some checks were not successful

1 failing and 1 successful checks
continuous-integration/travis-ci/push — The Travis CI build failed
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
5144759

@NegativeMjark NegativeMjark self-assigned this Nov 17, 2016

@matrixbot

matrixbot commented Nov 17, 2016

Can one of the admins verify this patch?

NegativeMjark added some commits Nov 17, 2016

@NegativeMjark
Merge remote-tracking branch 'origin/develop' into markjh/attachment_… 
…download

Some checks were not successful

1 failing and 1 successful checks
continuous-integration/travis-ci/push — The Travis CI build failed
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
2b854f6
@NegativeMjark
Comments, and only use the cross domain renderer if the attachment is… 
… encrypted

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
97ff11c
@NegativeMjark
Fix whitespace

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
dedf312
@NegativeMjark
Don't decrypt file attachments immediately

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
585ba30
@NegativeMjark
Merge remote-tracking branch 'origin/develop' into markjh/attachment_… 
…download

Conflicts:
	src/components/views/messages/MAudioBody.js
	src/components/views/messages/MFileBody.js
	src/components/views/messages/MImageBody.js
	src/components/views/messages/MVideoBody.js

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
08955ac

@NegativeMjark NegativeMjark referenced this pull request in vector-im/riot-web Nov 21, 2016

Closed

Support for large encrypted attachments #2615

@NegativeMjark
Contributor

NegativeMjark commented Nov 28, 2016

Needs vector-im/vector-web#2659 otherwise the download links will look awful.

@NegativeMjark NegativeMjark assigned dbkr and unassigned NegativeMjark Nov 28, 2016

@NegativeMjark
Use https://usercontent.riot.im/v1.html by default

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
7f00d9e
src/components/views/messages/MFileBody.js
+// ordinary javascript function which then is turned into a string using
+// toString().
+//
+const DEFAULT_CROSS_ORIGIN_RENDERER = "https://usercontent.riot.im/v1.html";
@NegativeMjark

NegativeMjark Nov 28, 2016

Contributor

This should probably be configurable. But I can't see a nice way to get values from vector-web's config.json to here.

@dbkr

dbkr reviewed Nov 29, 2016
View changes

Couple of comment typos. Agree on the usercontent location, wonder if there's a way to get the config option through nicely

src/components/views/messages/MFileBody.js
+// For attachments downloaded directly from the homeserver we can use
+// Content-Security-Policy headers to disable script execution.
+//
+// But attachments with end-to-end ecryption are more difficult to handle.
@dbkr

dbkr Nov 29, 2016

Member

ecryption

src/components/views/messages/MFileBody.js
+// the downside of using a sandboxed iframe is that the browers are overly
+// restrictive in what you are allowed to do with the generated URL.
+//
+// For now given how unusable the blobs generated in sandboxed iframes we'll
@dbkr

dbkr Nov 29, 2016

Member

how unusable the blobs generated in sandboxed iframes are ?

@NegativeMjark
typos

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
2ff73a2
src/components/views/messages/MImageBody.js
thumbnailPromise.then((thumbnailUrl) => {
- decryptFile(content.file).then((contentUrl) => {
+ decryptFile(content.file).then(function(blob) {
@Kegsay

Kegsay Dec 2, 2016

Contributor

This promise is being dropped to the floor if it rejects.

src/components/views/messages/MVideoBody.js
thumbnailPromise.then((thumbnailUrl) => {
- decryptFile(content.file).then((contentUrl) => {
+ decryptFile(content.file).then(function(blob) {
@Kegsay

Kegsay Dec 2, 2016

Contributor

This promise is being dropped to the floor if it rejects.

dbkr and others added some commits Dec 2, 2016

@dbkr
Put the config in the React context. 
Use it in MFileBody to configure the cross origin renderer URL.

All checks have passed

3 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
@matrixbot
default — Build finished.
Details
cd2a4b8
@dbkr
Call it appConfig in the context

All checks have passed

3 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
@matrixbot
default — Build finished.
Details
02dc570
@dbkr
Merge pull request #578 from matrix-org/dbkr/attachment_download_conf… 
…ig_context

Put the config in the React context.

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
28d7e6c
@NegativeMjark
Return the promises so they don't get dropped

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
faee976
@dbkr

dbkr approved these changes Dec 2, 2016
View changes

@NegativeMjark NegativeMjark merged commit 81e429e into develop Dec 2, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dbkr dbkr deleted the markjh/attachment_download branch Dec 14, 2016

Half-Shot added a commit to Half-Shot/matrix-react-sdk that referenced this pull request Feb 9, 2017

@NegativeMjark @Half-Shot
Fix e2e attachment download by using iframes. (#562) 
* Render attachments inside iframes.

* Fix up the image and video views

* Fix m.audio

* Comments, and only use the cross domain renderer if the attachment is encrypted

* Fix whitespace

* Don't decrypt file attachments immediately

* Use https://usercontent.riot.im/v1.html by default

* typos

* Put the config in the React context.

Use it in MFileBody to configure the cross origin renderer URL.

* Call it appConfig in the context

* Return the promises so they don't get dropped
5bdb055
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment