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

[xmpp] setup http_upload_external #1165

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

larchange
Copy link
Contributor

The problem

http upload is currently managed by metronome, but it's a deprecated part of metronome which has a limited mime types handling (For example pdf or doc file are not in the default handled mime types).

http upload is not the responsibility of metronome and need to be outsource.

Solution

use the external http upload way describe on metronome documentation.

PR Status

PENDING check/test/approval

How to test

  • Try to send a pdf file,
  • confirm the transfer failure
  • apply the patch
  • regenerate configurations for metronome and nginx
  • try to send again a pdf and other kinds of files
  • see the success .. or not !

@larchange
Copy link
Contributor Author

@maranda finally we may have external http upload
@pitchum here the PR for you to test :) Let me know what piece of code if any are not at the correct locations.

@larchange
Copy link
Contributor Author

maybe we can add
add_header Content-Security-Policy "default-src 'none'; frame-ancestors 'none';";

as advised in https://xmpp.org/extensions/xep-0363.html#server pointed by @randomstuff

@@ -14,6 +20,8 @@ do_pre_regen() {

# retrieve variables
main_domain=$(cat /etc/yunohost/current_host)
http_file_secret=$(cat /etc/yunohost/http_file_secret)
http_file_delete_secret=$(cat /etc/yunohost/http_file_delete_secret)
Copy link
Member

Choose a reason for hiding this comment

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

We probably need to rename those files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, what name do you propose ?

> "${metronome_conf_dir}/${domain}.cfg.lua"
done

# create the upload endpoint
[[ -d "/var/www/metronome/xmmp-upload" ]] \
|| mkdir -p /var/www/metronome/upload/
Copy link
Member

Choose a reason for hiding this comment

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

Seems a bug "xmpp-upload" vs "upload"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aie ! yes it's a bug... will change all these soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@zamentur
Copy link
Member

zamentur commented Mar 4, 2021

We should backup and restore the xmpp-upload dir and the secret you added.

@larchange
Copy link
Contributor Author

larchange commented Mar 4, 2021

We should backup and restore the xmpp-upload dir and the secret you added.

Do we really want to backup the xmpp-upload folder ? if we spawn a new instance somewhere else, metronome will not keep track of the files, and they will stay forever in the folder ...

@alexAubin alexAubin added the xmpp label Apr 2, 2021
@alexAubin alexAubin added this to In progress in 4.3.x via automation May 24, 2021
@zamentur zamentur moved this from In progress to Review in progress in 4.3.x Jun 1, 2021
@@ -0,0 +1,189 @@
<?php
Copy link
Member

@zamentur zamentur Jun 1, 2021

Choose a reason for hiding this comment

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

Do you know if equivalent script exist in python (we would avoid to add php in dependence of yunohost just for xmpp upload feature ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should not be complicated to rewrite it in python. I just took the official metronome example script. I can do it when I have time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we iterate instead of having the perfect PR ? Because this PR is quite useful.

@alexAubin alexAubin moved this from Review in progress to In progress in 4.3.x Sep 6, 2021
@alexAubin alexAubin removed this from Ongoing / to be tested/reviewed in 4.3.x Jan 20, 2022
@zamentur
Copy link
Member

zamentur commented Dec 1, 2022

I suggest to use this perl implementation instead https://github.com/weiss/ngx_http_upload . No new dependencies, no daemon to run.

There is also a flask implementation: https://github.com/horazont/xmpp-http-upload

In any case, Metronome should be a yunohost app, cause a lot of server don't use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants