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

Add Camo support #5386

Merged
merged 14 commits into from Nov 9, 2014

Conversation

Projects
None yet
2 participants
@denschub
Member

denschub commented Nov 9, 2014

This pull requests adds optional support for Camo. Camo is a small application to proxy insecure assets over secure channels (mainly markdown images). For more information, check out the wiki page.

The code might need some optimization, please review. Also I'm shamelessly using Travis to ensure my changes have no bad side effects, thus creating this PR. ;)

@jhass

This comment has been minimized.

Show comment
Hide comment
@jhass

jhass Nov 9, 2014

Member

Please don't call methods with :: :)

Member

jhass commented Nov 9, 2014

Please don't call methods with :: :)

@jhass

This comment has been minimized.

Show comment
Hide comment
@jhass

jhass Nov 9, 2014

Member

I don't quite follow why you added the processing to both, the getter methods and the message renderer. Are there still places where we hand out the content to the frontend through the getter methods instead of the message renderer?

The issue I see is that the getters are used when serializing the object for federation (https://github.com/diaspora/diaspora/blob/develop/app/models/comment.rb#L19).

Member

jhass commented Nov 9, 2014

I don't quite follow why you added the processing to both, the getter methods and the message renderer. Are there still places where we hand out the content to the frontend through the getter methods instead of the message renderer?

The issue I see is that the getters are used when serializing the object for federation (https://github.com/diaspora/diaspora/blob/develop/app/models/comment.rb#L19).

@jhass jhass added this to the next-major milestone Nov 9, 2014

Show outdated Hide outdated config/diaspora.yml.example
#proxy_remote_pod_images: true
## Root of your Camo installation
#root: "https://camo.example.com/"

This comment has been minimized.

@jhass

jhass Nov 9, 2014

Member

Total nitpick but I think the example should be https://example.com/camo/ since that's our recommended setup ;)

@jhass

jhass Nov 9, 2014

Member

Total nitpick but I think the example should be https://example.com/camo/ since that's our recommended setup ;)

This comment has been minimized.

@denschub

denschub Nov 9, 2014

Member

Indeed. Changed. ;)

@denschub

denschub Nov 9, 2014

Member

Indeed. Changed. ;)

Show outdated Hide outdated lib/diaspora/camo.rb
)
encoded_url = url.to_enum(:each_byte).map {|byte| '%02x' % byte}.join
"#{AppConfig.privacy.camo.root}#{digest}/#{encoded_url}"

This comment has been minimized.

@jhass

jhass Nov 9, 2014

Member

Little hack: File.join(AppConfig.privacy.camo.root, digest, encoded_url) so we don't depend on the trailing slash in the config.

@jhass

jhass Nov 9, 2014

Member

Little hack: File.join(AppConfig.privacy.camo.root, digest, encoded_url) so we don't depend on the trailing slash in the config.

This comment has been minimized.

@denschub

denschub Nov 9, 2014

Member

Oh, right, thanks!

@denschub

denschub Nov 9, 2014

Member

Oh, right, thanks!

denschub added some commits Nov 9, 2014

Method calling convention and doc improvements
... and a stealth-commit of an already introduced bug prevention system.
;)
@denschub

This comment has been minimized.

Show comment
Hide comment
@denschub

denschub Nov 9, 2014

Member

Are there still places where we hand out the content to the frontend through the getter methods instead of the message renderer?

Yeah, we had those. Anyways, I fixed the presenters and removed the getters to prevent federation issues. Thanks for the heads up!

Member

denschub commented Nov 9, 2014

Are there still places where we hand out the content to the frontend through the getter methods instead of the message renderer?

Yeah, we had those. Anyways, I fixed the presenters and removed the getters to prevent federation issues. Thanks for the heads up!

@jhass

This comment has been minimized.

Show comment
Hide comment
@jhass

jhass Nov 9, 2014

Member

Alright, let's try this. Thank you!

Member

jhass commented Nov 9, 2014

Alright, let's try this. Thank you!

jhass added a commit that referenced this pull request Nov 9, 2014

@jhass jhass merged commit a3a8a22 into diaspora:develop Nov 9, 2014

1 check passed

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

@denschub denschub deleted the denschub:camo branch Nov 9, 2014

@jhass jhass referenced this pull request Apr 26, 2015

Closed

mixed content #5894

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment