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

It would be better to place all the entries coming from the META tags, with an absolute URL. #651

Open
ZerooCool opened this issue Jun 23, 2020 · 17 comments · May be fixed by #668
Open

It would be better to place all the entries coming from the META tags, with an absolute URL. #651

ZerooCool opened this issue Jun 23, 2020 · 17 comments · May be fixed by #668

Comments

@ZerooCool
Copy link
Contributor

ZerooCool commented Jun 23, 2020

Perhaps it would be better to place all the entries coming from the META tags, with an absolute URL.

Thus, it would be necessary to add a variable, from the configuration file, which would be used to enter the domain name, and, thus, write the absolute URL address, in the files called from the META tags (images, files, css, js , json ...)

In this two files :

./tpl/page.php
./tpl/bootstrap.php

Important ! This issue was closed, but, only if this new issue #651 was resolved !

#646 ( See for documentation. )
#645
#644

@ZerooCool
Copy link
Contributor Author

ZerooCool commented Jun 23, 2020

In the ./lib/Controller.php we can found the VERSION value 1.3.4

In the tpl, we can use it :
<?php echo rawurlencode($VERSION); ?>

Then, we can add a value in the ./lib/Controller.php for add a HOST_VALUE and use it.

On the other hand, it would be better to be able to use the configuration file, because, I suppose that the administrator does not want to configure several files.

@rugk
Copy link
Member

rugk commented Jun 25, 2020

So would this really solve all the OpenGraph-related issues you've linked?
I just don't get why it does so?

But if it helps, we would be happy to appreciate a PR.

@ZerooCool
Copy link
Contributor Author

ZerooCool commented Jun 25, 2020

@rugk

What is a PR?
Yes I tested on my new deployment, and, putting the link in absolute URL effectively solves the problem.

(OLD) privatebin.visionduweb.fr
(NEW) privatebin.lecannabiste.com
Work !

How to : https://mediawiki.visionduweb.fr/index.php?title=Exporter_un_fichier_texte_vers_un_service_en_ligne_de_type_pastebin#Am.C3.A9liorer_le_rendu_pour_opengraph

Would it be a problem of chmod / chown, which prevents access if the url is relative in fact that I do not know.
Looking at some examples of sites that work for opengraph, the url is always an absolute url.

In my case, I modified the code in the template myself.

As indicated in the open message, to correct this problem, you would have to create a configuration variable, to be able to enter your domain name, and, that this variable be used in the template.
I don't know how to link a variable from the configuration file to the template.

On the other hand, I identified the way of doing it, if the variable domain is present in the library file ./lib/Controller.php as I indicated, since it seems that it is this file which informs about the value 1.3 .4 which is used in the template, following the images.

I have not tested here but I wonder if we fill in "localhost" if it could work, but, in fact, no I don't think so, because the absolute url would then be localhost / image.ext and I suppose that l will not be found, since Twitter or Facebook will request from outside, to localhost / image.ext

So, in my opinion, the only way is to be able to clearly indicate the domain name used.

The best for user comfort, is to know how to add such a variable in the configuration file, rather than in the library file, otherwise, two files are to be filled in, and, we get lost in the configuration (for the administrator from the server.)

The removal of version 1.3.4 does not seem necessary, contrary to what I thought, thus, the PHP tags can remain in place behind the image. On the other hand, it seems that Facebook does not like that, when checking the opengraph validity of the image, but, it works.

https://developers.facebook.com/tools/debug/?q=https%3A%2F%2Fprivatebin.lecannabiste.com

It even seems that Facebook no longer mentions the alert for 1.3.4, so it would seem that this alert was more an alert related to the missing domain name, therefore, a relative URL that facebook could not match .

Thus, the answer to the opengraph problem is to define an absolute URL, with HTTP / HTTPS ...

@rugk
Copy link
Member

rugk commented Jun 29, 2020

What is a PR?

For information on how to propose PRs, see https://help.github.com/articles/proposing-changes-to-your-work-with-pull-requests/. See this article for how to merge upstream changes into your fork.

This way, you can propose us changes you want to contribute to this project (called "upstream").
Please also read our contributing guide to see how to get started with development and help us: https://github.com/PrivateBin/PrivateBin/blob/master/.github/CONTRIBUTING.md

@ZerooCool
Copy link
Contributor Author

Sorry, Pull Request * OK.

Yes, I have already done a PR for the language.

But, for this URL problem, as I said, the best would be to be able to use the configuration file, to indicate the domain name, but, I don't know how, to use the variable of the configuration file in the PHP code of the template.

On the other hand, it is possible to get around my lack of knowledge, by using the library file, to indicate the domain name there, but, I find it much less comfortable for the administrator, who will have to modify 2 configuration files at place of one.

Another solution would then be to comment out the Opengraph code, since it does not work, and then to indicate in the comment that it is necessary to indicate an absolute URL for the image ...
In any case, the code currently proposed is NOT functional, for the opengraph.

@rugk
Copy link
Member

rugk commented Jun 29, 2020

but, I don't know how, to use the variable of the configuration file in the PHP code of the template.

This may help you: https://github.com/PrivateBin/PrivateBin/wiki/Development#creating-new-configuration-options

@ZerooCool
Copy link
Contributor Author

ZerooCool commented Jun 30, 2020

Thank you for the link, well in fact, I do not make an object, so little, that, your resource did not help me at all, on the contrary: /

Finally, I did a search on the keyword notice, which is not very present, to see how to steal his behavior.

I don't know if I'm using the best way, since notice is text, and in my case it's a URL. (which could be filtered, in the best of cases, but hey, the administrator is supposed to have correctly filled in his own URL, and, doing a test during the production.)

sudo nano PrivateBin/lib/Configuration.php

# Default value
 private static $_defaults = array(
        'main' => array(
            'name'                     => 'PrivateBin',
            'domain'                   => 'AHH ITS MY DOMAIN',

sudo nano PrivateBin/lib/Controller.php

# Same line, example used from notice ! Just or not ?
        $page = new View;
        $page->assign('NAME', $this->_conf->getKey('name'));
        $page->assign('DOMAIN', I18n::_($this->_conf->getKey('domain')));

sudo nano PrivateBin/cfg/conf.sample.php

# Example configuration.
# Need rename : sudo cp conf.sample.php conf.php
# Remove ; and then, the new adresse work !
;domain = "https://my-real-new-domain.ext"

sudo nano PrivateBin/tpl/bootstrap.php

# Then i can write the domain in the bootstrap.php file : 
<?php echo I18n::encode($DOMAIN), PHP_EOL; ?>

Thus, I manage to write the variable.
Before making a PR, could you confirm to me that it is correct to have used the same model as NOTICE which is text.

Thank you.

@rugk
Copy link
Member

rugk commented Jun 30, 2020

Looks good, maybe rename it to "path" or so and explain in the config that this only applies to the meta tags, but except of that it looks good.

@ZerooCool
Copy link
Contributor Author

Ok. I look.

@ZerooCool
Copy link
Contributor Author

ZerooCool commented Jun 30, 2020

Test OK.
I remove this code, because, Opengraph doesn't work with this :
?<?php echo rawurlencode($VERSION); ?>

I up to my repo and send a PR : #664

ZerooCool added a commit to ZerooCool/PrivateBin that referenced this issue Jul 1, 2020
Make Opengraph really functional

Change : PrivateBin#664 for PrivateBin#651
@ZerooCool
Copy link
Contributor Author

I add the line with BASEPATH in tst/ViewTest.php

        $page = new View;
        $page->assign('NAME', 'PrivateBinTest');
        $page->assign('BASEPATH', false);

@ZerooCool
Copy link
Contributor Author

ZerooCool commented Jul 1, 2020

Note, i can't use ", PHP_EOL" otherwise, the rest of the code is returned on a new line. So I simply removed ", PHP_EOL" without certainty on the correct syntax to use in the rules of art. It works that way :

sudo nano PrivateBin/tpl/bootstrap.php

Then i can write the domain in the bootstrap.php file :

@elrido
Copy link
Contributor

elrido commented Jul 3, 2020

PHP_EOL is one of PHPs predefined constants which contains the "correct 'End Of Line' symbol for this platform." - That is a line-feed-character on Linux and UNIX (incl. MacOS since version 10) and a carriage-return-character on Windows.

We use that pattern in the templates because PHP tries to be clever and removes the line feed characters after you end a PHP block with ?>. To avoid making the HTML less legible, we add a PHP_EOL at the end of an echo to ensure a line break is preserved.

In your case, where you want to output your variable in the same line as the path to the image, it is correct to not add that constant.

@ZerooCool
Copy link
Contributor Author

Close with #651 and PR #668

@rugk
Copy link
Member

rugk commented Jul 6, 2020

Nah, let's only close this when the PR to fix this is merged.

You can (automatically) let issues close when a PR is merged by adding some "magic" text to your PR body, so manually closing is not needed.

@rugk rugk reopened this Jul 6, 2020
@ZerooCool
Copy link
Contributor Author

ZerooCool commented Jul 10, 2020

It seemed to me that the PR had been integrated here, I believe that it is the case: #664
But this issue is always open : #668

@ZerooCool
Copy link
Contributor Author

ZerooCool commented Aug 10, 2020

UP - Last version from Privatebin, from Github, today.

Opengraph doesn't work !

https://privatebin.monnaie-libre.fr

URL is not with absolute path.

Making too good PHP code ... Makes it not work.
My PR allowed it to run OpenGraph.
Can you see why the absolute url of Opengraph images is not working?
PR was push or not ?

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