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

Remove trailing slash from YOURLS_SITE #2244

Open
ozh opened this Issue May 2, 2017 · 21 comments

Comments

Projects
None yet
3 participants
@ozh
Member

ozh commented May 2, 2017

We should be more forgiving with noobs who cannot RTFM (or maybe don't understand English) and define their YOURLS_SITE with a trailing slash (eg http://sho.rt/ instead of http://sho.rt as advised)

This is a very minor bug but it produces discrepancies in links, href and src's. For instance javascript and stylesheets are included correctly (<script src="https://sho.rt/js/jquerybleh") but some other links are not (var ajaxurl = 'https://sho.rt//admin/admin-ajax.php'; or internal links such as <a href="https://sho.rt//admin/tools.php">)

For the record, roughly 4% (!!!) of YOURLS users misconfigure this variable with a trailing slash.

ozh added a commit that referenced this issue May 2, 2017

PopVeKind referenced this issue in PopVeKind/YOURLS Dec 3, 2017

MultiSite - Split YOURLS-SITE + Autodetect - Add MultiSite Config Files
This Patch Prepares the way for true MultiSite.  It automaticly defines
YOURLS_SITE (used to make shortened URLS and YOURLS_ABSURL the absolute
URL for the site files. And adds MultiSite Config.
@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 8, 2017

Contributor

After careful consideration...

After careful consideration, I might suggest removing YOURLS_SITE from the config file of most sites!

The small patch Splitting YOURLS_SITE - Added Features #2335 automatically calculates YOURLS_SITE so noobs don't have to do it (see number 1 on the List of Benefits in this comment)

Contributor

PopVeKind commented Dec 8, 2017

After careful consideration...

After careful consideration, I might suggest removing YOURLS_SITE from the config file of most sites!

The small patch Splitting YOURLS_SITE - Added Features #2335 automatically calculates YOURLS_SITE so noobs don't have to do it (see number 1 on the List of Benefits in this comment)

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Dec 8, 2017

Member

What about those who don't fall into the "most sites" category?

Member

ozh commented Dec 8, 2017

What about those who don't fall into the "most sites" category?

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 8, 2017

Contributor

In-depth Information

In the same comment (see number 8 on the List of Benefits), I propose an Admin Diag Page Plugin (Noob Friendly!) Personally, I believe this should in the core for all to use.

  • Adds a diagnostic tab to the admin area

  • Helps user fix the trailing edge slash configuration error

  • Provides Technical Details: A list, regarding the environment, to copy/paste into issues - like server version, PHP version, database version, plus constants like YOURLS_ABSURL and YOURLS_SITE Examples:
    Apache Ver = 2.x.x
    PHP Ver = 7.0.2
    MariaDB Ver = x.x.x
    YOURLS_ABSURL = https://sho.rt/yourls/
    YOURLS_SITE = https://sho.rt - Perfect!
    ~ ~ ~ OR ~ ~ ~
    YOURLS_SITE = https://sho.rt/y/ - WARNING: Remove the last (right) slash. (in Red)
    ~ ~ ~ OR ~ ~ ~
    YOURLS_SITE = https://sho.rt/y - WARNING: Subdirectory sites must be in the same phyical directory. Rename the /yourls/ directory to /y/ and adjust .htaccess as follows... (in Red)

  • Fixing Noob Errors: Offers easy advice!
    Examples:
    YOURLS_DB_PASS = your db password - WARNING: Check or Correct This. (in Red)
    YOURLS_DB_PREFIX - MISSING : use config-sample.php to create a new config file. (in Red)

  • Best Practices: Gives advice like how to convert from a single site to Multisite.
    (This assumes the MultiSite Plugin is activated and the site domain name is sho.rt - this is automatically detected).
    Example:
    For MultiSite: Rename config.php to sho.rt-config.php in the /user/ directory.

Contributor

PopVeKind commented Dec 8, 2017

In-depth Information

In the same comment (see number 8 on the List of Benefits), I propose an Admin Diag Page Plugin (Noob Friendly!) Personally, I believe this should in the core for all to use.

  • Adds a diagnostic tab to the admin area

  • Helps user fix the trailing edge slash configuration error

  • Provides Technical Details: A list, regarding the environment, to copy/paste into issues - like server version, PHP version, database version, plus constants like YOURLS_ABSURL and YOURLS_SITE Examples:
    Apache Ver = 2.x.x
    PHP Ver = 7.0.2
    MariaDB Ver = x.x.x
    YOURLS_ABSURL = https://sho.rt/yourls/
    YOURLS_SITE = https://sho.rt - Perfect!
    ~ ~ ~ OR ~ ~ ~
    YOURLS_SITE = https://sho.rt/y/ - WARNING: Remove the last (right) slash. (in Red)
    ~ ~ ~ OR ~ ~ ~
    YOURLS_SITE = https://sho.rt/y - WARNING: Subdirectory sites must be in the same phyical directory. Rename the /yourls/ directory to /y/ and adjust .htaccess as follows... (in Red)

  • Fixing Noob Errors: Offers easy advice!
    Examples:
    YOURLS_DB_PASS = your db password - WARNING: Check or Correct This. (in Red)
    YOURLS_DB_PREFIX - MISSING : use config-sample.php to create a new config file. (in Red)

  • Best Practices: Gives advice like how to convert from a single site to Multisite.
    (This assumes the MultiSite Plugin is activated and the site domain name is sho.rt - this is automatically detected).
    Example:
    For MultiSite: Rename config.php to sho.rt-config.php in the /user/ directory.

@majordome

This comment has been minimized.

Show comment
Hide comment
@majordome

majordome bot Dec 8, 2017

Thanks for your comment, @PopVeKind!

Your reply is very long.
As a bot, I take less than a second to read it, but summing it up a little bit
will help old-fashioned humans understand your thought.

majordome bot commented Dec 8, 2017

Thanks for your comment, @PopVeKind!

Your reply is very long.
As a bot, I take less than a second to read it, but summing it up a little bit
will help old-fashioned humans understand your thought.

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Dec 9, 2017

Member

What majordome says.

Member

ozh commented Dec 9, 2017

What majordome says.

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 9, 2017

Contributor

Good question @ozh - these would be those in the advanced user category! :)

Short Answer

  1. https in shorten URLs and
  2. subdirectory installs.

I'm slightly opposed to using code bloat, on every request, to catch config errors. In the install instructions, I would direct users to a diagnostic tab in the Admin area for checking their install (the final step in the install and a must use step before writing a support issue).

Long Answer - See In-depth Information

Contributor

PopVeKind commented Dec 9, 2017

Good question @ozh - these would be those in the advanced user category! :)

Short Answer

  1. https in shorten URLs and
  2. subdirectory installs.

I'm slightly opposed to using code bloat, on every request, to catch config errors. In the install instructions, I would direct users to a diagnostic tab in the Admin area for checking their install (the final step in the install and a must use step before writing a support issue).

Long Answer - See In-depth Information

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Dec 10, 2017

Member

I don't understand what you mean with "https in shorten URLs" regarding users for which all server vars $_SERVER, on which you heavily rely, don't populate as expected

Member

ozh commented Dec 10, 2017

I don't understand what you mean with "https in shorten URLs" regarding users for which all server vars $_SERVER, on which you heavily rely, don't populate as expected

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 11, 2017

Contributor

@ozh

"https in shorten URLs"

https://sh.ort/abc vs http://sh.ort/abc This is not about admin pages.

HTTPS would still require YOURLS_SITE to be defined. I'd consider HTTPS an advanced user configuration. From issues I have read, it seems HTTP is used the majority of the time.

This patch uses YOURLS_SITE only in the formation of short URLs. I changed the "YOURLS installation URL" part of the purpose.

YOURLS_ABSURL becomes the "YOURLS installation URL" This allows backward compatibility for config.php users with YOURLS_ABSURL defined. I changed the "YOURLS installation URL" part of the purpose

Splitting the two purposes of YOURLS_SITE is absolutely required for many features, including multisite. Changing the "formation of short URLs" purpose of YOURLS_SITE would have made a smaller patch. It
would also force every admin to modify their config.php - I do not consider that acceptable.

I would suggest catching the trailing slash error with a diag page in the admin area, not in bloated active code.

Contributor

PopVeKind commented Dec 11, 2017

@ozh

"https in shorten URLs"

https://sh.ort/abc vs http://sh.ort/abc This is not about admin pages.

HTTPS would still require YOURLS_SITE to be defined. I'd consider HTTPS an advanced user configuration. From issues I have read, it seems HTTP is used the majority of the time.

This patch uses YOURLS_SITE only in the formation of short URLs. I changed the "YOURLS installation URL" part of the purpose.

YOURLS_ABSURL becomes the "YOURLS installation URL" This allows backward compatibility for config.php users with YOURLS_ABSURL defined. I changed the "YOURLS installation URL" part of the purpose

Splitting the two purposes of YOURLS_SITE is absolutely required for many features, including multisite. Changing the "formation of short URLs" purpose of YOURLS_SITE would have made a smaller patch. It
would also force every admin to modify their config.php - I do not consider that acceptable.

I would suggest catching the trailing slash error with a diag page in the admin area, not in bloated active code.

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 11, 2017

Contributor

Special Syntax

In config.php YOURLS_SITE is the ONLY constant that has a Special Syntax!

The Special Syntax of YOURLS_SITE is what causes the trailing slash error.

Contributor

PopVeKind commented Dec 11, 2017

Special Syntax

In config.php YOURLS_SITE is the ONLY constant that has a Special Syntax!

The Special Syntax of YOURLS_SITE is what causes the trailing slash error.

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 11, 2017

Contributor

Shop Rules for Config Files.

The shops I worked in had rules for config files.

Defined constants must be defined with either:

  • No Syntax
  • Copy/Paste Syntax

No Syntax

Consider defining the parts of YOURLS_SITE https://sho.rt/y into three constants:

  • http or https (protocol)
  • sho.rt (domain)
  • y (directory)

Although this method has three constants instead of one, without Special Syntax, it is much less likely to be wrong. YOURLS could adopt this even now, with cleaner results like an admin protocol override or directory.

It sounds like more options = more complexity. However, most actual errors in config files are caused by Special Syntax.

Copy/Paste Syntax

The exception to the above are things which are copy/paste.

  1. Type edition.cnn.com into a browser (I use Firefox and Chrome), and view the site.
  2. Highlight and Copy edition.cnn.com from the browser address bar.
  3. Paste into a text editor.
  4. Results: http://edition.cnn.com/ - FAIL! There is a trailing slash! Noobs who cannot RTFM?
Contributor

PopVeKind commented Dec 11, 2017

Shop Rules for Config Files.

The shops I worked in had rules for config files.

Defined constants must be defined with either:

  • No Syntax
  • Copy/Paste Syntax

No Syntax

Consider defining the parts of YOURLS_SITE https://sho.rt/y into three constants:

  • http or https (protocol)
  • sho.rt (domain)
  • y (directory)

Although this method has three constants instead of one, without Special Syntax, it is much less likely to be wrong. YOURLS could adopt this even now, with cleaner results like an admin protocol override or directory.

It sounds like more options = more complexity. However, most actual errors in config files are caused by Special Syntax.

Copy/Paste Syntax

The exception to the above are things which are copy/paste.

  1. Type edition.cnn.com into a browser (I use Firefox and Chrome), and view the site.
  2. Highlight and Copy edition.cnn.com from the browser address bar.
  3. Paste into a text editor.
  4. Results: http://edition.cnn.com/ - FAIL! There is a trailing slash! Noobs who cannot RTFM?
@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 11, 2017

Contributor

$_SERVER Doesn't Populate Correctly!

As a University Professor, I cannot tell you the hundreds or even thousands of times I have heard this. You can read this all over the Internet, even on php.net! However, it is simply not true!

What people actually mean is Apache, IIS, and Nginx may populate $_SERVER differently.
To quote you,

$_SERVER, on which you heavily rely, don't populate as expected

Expected by who? Did they actually read the Web Server Manuals? Or did they expect it because someone said it somewhere on some website, and it was quoted by a million other people?

Because you are talking about me, and the code I "heavily rely" on, I assume we are talking about $_SERVER being populated as I expect it! I'm not a noob. I expect $_SERVER to be populated with the environmental server vars passed to PHP from the web server. I expect Apache to pass Apache vars, IIS to pass IIS vars and Nginx to pass Nginx vars - as defined in their respective manuals. Likewise, I never expect Apache or Nginx to pass IIS vars!

Of course, different servers may pass different (confusing) vars into PHP. A self-respecting coder would structure his code to not allow this to change the output from one server to another. Here is an example:

  1. $_SERVER[ 'HTTPS' ] is empty (does not exist or value equals FALSE) when Nginx detects HTTP.
  2. The same condition (HTTP) in IIS is equal to TRUE and "off"!
  3. In Apache, this returns '0' 'off' or FALSE.
  • Obviously empty or 'off' means this test is not HTTPS and further tests are needed.
  • Likewise, 'on' or '1' after !== 'off' is always conclusive proof of HTTPS.
  • Additionally, TRUE, FALSE, and '0' cannot determine if this test is HTTPS and further tests are needed.
if ( !empty( $_SERVER[ 'HTTPS' ] ) && $_SERVER[ 'HTTPS' ] !== 'off' ) {
    // good in IIS and Apache
    if ( 'on' == strtolower( $_SERVER[ 'HTTPS' ] ) ) define( 'YOURLS_PROTOCOL', 'https://' );
    if ( '1' == $_SERVER[ 'HTTPS' ] ) define( 'YOURLS_PROTOCOL', 'https://' );
}

$_SERVER is Populated Correctly and I can rely on results expected, based on the server manules.

I will absolutely agree with you that there are some "noobs who cannot RTFM!" I had plenty in my PHP classes who thought all they needed was the PHP manual. They never even consulted the manuals for IIS, Apache, Nginx, etc, to see what was being populated in $_SERVER.

Contributor

PopVeKind commented Dec 11, 2017

$_SERVER Doesn't Populate Correctly!

As a University Professor, I cannot tell you the hundreds or even thousands of times I have heard this. You can read this all over the Internet, even on php.net! However, it is simply not true!

What people actually mean is Apache, IIS, and Nginx may populate $_SERVER differently.
To quote you,

$_SERVER, on which you heavily rely, don't populate as expected

Expected by who? Did they actually read the Web Server Manuals? Or did they expect it because someone said it somewhere on some website, and it was quoted by a million other people?

Because you are talking about me, and the code I "heavily rely" on, I assume we are talking about $_SERVER being populated as I expect it! I'm not a noob. I expect $_SERVER to be populated with the environmental server vars passed to PHP from the web server. I expect Apache to pass Apache vars, IIS to pass IIS vars and Nginx to pass Nginx vars - as defined in their respective manuals. Likewise, I never expect Apache or Nginx to pass IIS vars!

Of course, different servers may pass different (confusing) vars into PHP. A self-respecting coder would structure his code to not allow this to change the output from one server to another. Here is an example:

  1. $_SERVER[ 'HTTPS' ] is empty (does not exist or value equals FALSE) when Nginx detects HTTP.
  2. The same condition (HTTP) in IIS is equal to TRUE and "off"!
  3. In Apache, this returns '0' 'off' or FALSE.
  • Obviously empty or 'off' means this test is not HTTPS and further tests are needed.
  • Likewise, 'on' or '1' after !== 'off' is always conclusive proof of HTTPS.
  • Additionally, TRUE, FALSE, and '0' cannot determine if this test is HTTPS and further tests are needed.
if ( !empty( $_SERVER[ 'HTTPS' ] ) && $_SERVER[ 'HTTPS' ] !== 'off' ) {
    // good in IIS and Apache
    if ( 'on' == strtolower( $_SERVER[ 'HTTPS' ] ) ) define( 'YOURLS_PROTOCOL', 'https://' );
    if ( '1' == $_SERVER[ 'HTTPS' ] ) define( 'YOURLS_PROTOCOL', 'https://' );
}

$_SERVER is Populated Correctly and I can rely on results expected, based on the server manules.

I will absolutely agree with you that there are some "noobs who cannot RTFM!" I had plenty in my PHP classes who thought all they needed was the PHP manual. They never even consulted the manuals for IIS, Apache, Nginx, etc, to see what was being populated in $_SERVER.

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 11, 2017

Contributor

@ozh

It might interest you to know that Define YOURLS_PROTOCOL was written and thoroughly tested, on different web servers, over a six month period by 18 experienced PHP programmers working together.

The need for this was due to an unreliable WordPress function is_ssl(), also based on $_SERVER. This function fails in many places, like behind load balancers and reverse proxies, and in some IIS HTTPS servers.

Much of this faulty WP function now exist in function yourls_is_ssl() It likely doesn't matter on basic Apache and Nginx servers. YOURLS is already relying on $_SERVER.

Contributor

PopVeKind commented Dec 11, 2017

@ozh

It might interest you to know that Define YOURLS_PROTOCOL was written and thoroughly tested, on different web servers, over a six month period by 18 experienced PHP programmers working together.

The need for this was due to an unreliable WordPress function is_ssl(), also based on $_SERVER. This function fails in many places, like behind load balancers and reverse proxies, and in some IIS HTTPS servers.

Much of this faulty WP function now exist in function yourls_is_ssl() It likely doesn't matter on basic Apache and Nginx servers. YOURLS is already relying on $_SERVER.

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 14, 2017

Contributor

Patch Solution

@ozh - I believe we are very near an acceptable solution regarding this patch. It appears this is now just a matter of opinion, so let's work out a way forward.

Opinions

  1. For more than 15 years I have demonstrated that $_SERVER elements are 100% reliable and populate as expected, based on the web-server manuals (apache, etc.).
  1. I won't speak for your opinion. However, it does seem fair to say you appear apprehensive to use all $_SERVER elements because you believe some "don't populate as expected."

THEREFORE, we should rewrite the patch to remove as many $_SERVER elements as possible.

Getting Help

My former student offered to post this on the Alumni Newsletter (about 3,000 working programmers who, at one point or another, were my students in University). It would be helpful if you would first approve some ground rules.

Proposed Ground Rules

  1. Avoid Code Bloat, as much as possible, by putting new features into Plugins.
  2. $_SERVER elements already used in the YOURLS Core Code are considered safe to use.
  3. $_SERVER elements currently used in the WordPress Core Code are considered safe to use. (75 Million WordPress websites have thoroughly tested these,)
  4. Reduce or eliminate other $_SERVER elements as much as possible.
  5. For security, I would also like permission to freeze the $_SERVER['HTTP_HOST'] with the following code in the early part of Config like this...
    // freeze the HTTP_HOST
    if ( !defined( 'YOURLS_HTTP_HOST' ) )
        define( 'YOURLS_HTTP_HOST', $_SERVER[ 'HTTP_HOST' ] );
Contributor

PopVeKind commented Dec 14, 2017

Patch Solution

@ozh - I believe we are very near an acceptable solution regarding this patch. It appears this is now just a matter of opinion, so let's work out a way forward.

Opinions

  1. For more than 15 years I have demonstrated that $_SERVER elements are 100% reliable and populate as expected, based on the web-server manuals (apache, etc.).
  1. I won't speak for your opinion. However, it does seem fair to say you appear apprehensive to use all $_SERVER elements because you believe some "don't populate as expected."

THEREFORE, we should rewrite the patch to remove as many $_SERVER elements as possible.

Getting Help

My former student offered to post this on the Alumni Newsletter (about 3,000 working programmers who, at one point or another, were my students in University). It would be helpful if you would first approve some ground rules.

Proposed Ground Rules

  1. Avoid Code Bloat, as much as possible, by putting new features into Plugins.
  2. $_SERVER elements already used in the YOURLS Core Code are considered safe to use.
  3. $_SERVER elements currently used in the WordPress Core Code are considered safe to use. (75 Million WordPress websites have thoroughly tested these,)
  4. Reduce or eliminate other $_SERVER elements as much as possible.
  5. For security, I would also like permission to freeze the $_SERVER['HTTP_HOST'] with the following code in the early part of Config like this...
    // freeze the HTTP_HOST
    if ( !defined( 'YOURLS_HTTP_HOST' ) )
        define( 'YOURLS_HTTP_HOST', $_SERVER[ 'HTTP_HOST' ] );
@majordome

This comment has been minimized.

Show comment
Hide comment
@majordome

majordome bot Dec 14, 2017

Thanks for your comment, @PopVeKind!

Your reply is very long.
As a bot, I take less than a second to read it, but summing it up a little bit
will help old-fashioned humans understand your thought.

majordome bot commented Dec 14, 2017

Thanks for your comment, @PopVeKind!

Your reply is very long.
As a bot, I take less than a second to read it, but summing it up a little bit
will help old-fashioned humans understand your thought.

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 14, 2017

Contributor

Patch Rewrite

@ozh - I think we are near an acceptable solution regarding this patch. Based on your comment we should rewrite the patch to remove as many $_SERVER elements as possible. I will have my former students look into this. I would like your approval of these three Instructions I will include for them.

Proposed Rewrite Instructions

  1. Avoid Code Bloat, as much as possible, by putting new features into Plugins.
  2. $_SERVER elements used in the WordPress or YOURLS Core Code are considered safe to use.
    (75 Million WordPress websites have thoroughly tested these,)
  3. May we have permission to freeze the HTTP_HOST with the following code in the early part of Config.
    // freeze the HTTP_HOST
    if ( !defined( 'YOURLS_HTTP_HOST' ) )
        define( 'YOURLS_HTTP_HOST', $_SERVER[ 'HTTP_HOST' ] );
Contributor

PopVeKind commented Dec 14, 2017

Patch Rewrite

@ozh - I think we are near an acceptable solution regarding this patch. Based on your comment we should rewrite the patch to remove as many $_SERVER elements as possible. I will have my former students look into this. I would like your approval of these three Instructions I will include for them.

Proposed Rewrite Instructions

  1. Avoid Code Bloat, as much as possible, by putting new features into Plugins.
  2. $_SERVER elements used in the WordPress or YOURLS Core Code are considered safe to use.
    (75 Million WordPress websites have thoroughly tested these,)
  3. May we have permission to freeze the HTTP_HOST with the following code in the early part of Config.
    // freeze the HTTP_HOST
    if ( !defined( 'YOURLS_HTTP_HOST' ) )
        define( 'YOURLS_HTTP_HOST', $_SERVER[ 'HTTP_HOST' ] );
@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Dec 14, 2017

Member

To fix the trailing slash issue, I was more on something trivial like trimming YOURLS_SITE whenever it's used, not introducing yet another constant. There are already too much consts in YOURLS. Constants are nearly impossible to test in unit tests, and they cannot be modified by plugins.

Member

ozh commented Dec 14, 2017

To fix the trailing slash issue, I was more on something trivial like trimming YOURLS_SITE whenever it's used, not introducing yet another constant. There are already too much consts in YOURLS. Constants are nearly impossible to test in unit tests, and they cannot be modified by plugins.

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 14, 2017

Contributor

I see what you mean about unit tests.

  1. Wouldn't it be better to add a diagnostic page to the admin area and fix the problem?

On install, it would help noobs get it right!
Run before support request issues, it could catch all kinds of errors and give solutions (reduce support requests) and provide a copy/paste into issues for environments like PHP Version, etc.

This would allow existing error checking code, that runs on every page load, to be removed. The result would be some small performance boost and less error checking code bloat.

  1. Another idea would be to build an install page, like WordPress uses, to build the config.php file.

Either or both of these would be easy enough to build.

Contributor

PopVeKind commented Dec 14, 2017

I see what you mean about unit tests.

  1. Wouldn't it be better to add a diagnostic page to the admin area and fix the problem?

On install, it would help noobs get it right!
Run before support request issues, it could catch all kinds of errors and give solutions (reduce support requests) and provide a copy/paste into issues for environments like PHP Version, etc.

This would allow existing error checking code, that runs on every page load, to be removed. The result would be some small performance boost and less error checking code bloat.

  1. Another idea would be to build an install page, like WordPress uses, to build the config.php file.

Either or both of these would be easy enough to build.

@LeoColomb

This comment has been minimized.

Show comment
Hide comment
@LeoColomb

LeoColomb Dec 14, 2017

Member

build an install page

#1254

Either or both of these would be easy enough to build.

Let's do a Pull Request! 😄 🎉

Member

LeoColomb commented Dec 14, 2017

build an install page

#1254

Either or both of these would be easy enough to build.

Let's do a Pull Request! 😄 🎉

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Dec 29, 2017

Contributor

We should be more forgiving with noobs - ozh

To really help noobs the most, we should move the core config options from config.php to the database. A config page in the admin area could then ask human questions, that could be selected with a mouse click, and be properly stored in a machine-friendly format. Here is a work-in-progress idea for noobs.

Trivial Trimming

To fix the trailing slash issue, I was more on something trivial like trimming YOURLS_SITE whenever it's used, not introducing yet another constant. - ozh

By my count, there are 42 Places YOURLS_SITE is used. Meaning 42 places we would need to add the trim function. It also means, when we move YOURLS_SITE to the DB, those 42 trim functions will not be needed and will contribute to Code Bloat!

A Cleaner Trim

I suggest, for noobs, we rename YOURLS_SITE to YOURLS_SITE_URL and keep YOURLS_SITE in config.php TEMPORARILY, as we transition to putting the Config settings in the DB. The Config class could trim YOURLS_SITE to define YOURLS_SITE_URL.

YOURLS_SITE (in config.php) would retire forever when moved into the DB.

Split YOURLS_SITE Patch

$_SERVER, on which you heavily rely, don't populate as expected - ozh

I am currently making tests, and final adjustments, on the latest version of the Split YOURLS_SITE Patch #2335. Per @ozh comment above, this version does not heavily rely on $_SERVER Eleven of the 42 YOURLS_SITE constants will be converted to YOURLS_ABS_URL and populated automatically. If you want, I could easily change the other 31 to YOURLS_SITE_URL, as in A Cleaner Trim transition (above).

  • YOURLS_SITE = Transition to DB to be retired.
  • YOURLS_SITE_URL = For building short URLs (Ex. http://sho.rt)
  • YOURLS_ABS_URL = For building links to files (Ex. http://sho.rt/yourls) Needed for MultiSite and other Plugins as stated in #2335
Contributor

PopVeKind commented Dec 29, 2017

We should be more forgiving with noobs - ozh

To really help noobs the most, we should move the core config options from config.php to the database. A config page in the admin area could then ask human questions, that could be selected with a mouse click, and be properly stored in a machine-friendly format. Here is a work-in-progress idea for noobs.

Trivial Trimming

To fix the trailing slash issue, I was more on something trivial like trimming YOURLS_SITE whenever it's used, not introducing yet another constant. - ozh

By my count, there are 42 Places YOURLS_SITE is used. Meaning 42 places we would need to add the trim function. It also means, when we move YOURLS_SITE to the DB, those 42 trim functions will not be needed and will contribute to Code Bloat!

A Cleaner Trim

I suggest, for noobs, we rename YOURLS_SITE to YOURLS_SITE_URL and keep YOURLS_SITE in config.php TEMPORARILY, as we transition to putting the Config settings in the DB. The Config class could trim YOURLS_SITE to define YOURLS_SITE_URL.

YOURLS_SITE (in config.php) would retire forever when moved into the DB.

Split YOURLS_SITE Patch

$_SERVER, on which you heavily rely, don't populate as expected - ozh

I am currently making tests, and final adjustments, on the latest version of the Split YOURLS_SITE Patch #2335. Per @ozh comment above, this version does not heavily rely on $_SERVER Eleven of the 42 YOURLS_SITE constants will be converted to YOURLS_ABS_URL and populated automatically. If you want, I could easily change the other 31 to YOURLS_SITE_URL, as in A Cleaner Trim transition (above).

  • YOURLS_SITE = Transition to DB to be retired.
  • YOURLS_SITE_URL = For building short URLs (Ex. http://sho.rt)
  • YOURLS_ABS_URL = For building links to files (Ex. http://sho.rt/yourls) Needed for MultiSite and other Plugins as stated in #2335
@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Apr 9, 2018

Contributor

Moving the Core Options to the database would automatically fix this, see
DB Config Page and Installer Project #2380

Also, notice the Ten Core Constants this Retires (eliminates), and how they are replaced in the DB ($ydb) at the bottom of this working demo page...
http://a.fil.net/s/admin/db-config.php?dev=notes

  • YOURLS_SITE is eliminated and so is the trailing slash problem.

Core Constants eliminated include:

  1. YOURLS_HOURS_OFFSET (eliminated DST/Summertime offset problem)
  2. YOURLS_LANG
  3. YOURLS_UNIQUE_URLS
  4. YOURLS_PRIVATE
  5. YOURLS_COOKIEKEY
  6. YOURLS_URL_CONVERT
  7. YOURLS_DEBUG
  8. $yourls_reserved_URL (array)
  9. YOURLS_ADMIN_SSL (https code bloat)
  10. YOURLS_SITE (trailing slash problem)

Constants are a problem for many plugins under developement.

Contributor

PopVeKind commented Apr 9, 2018

Moving the Core Options to the database would automatically fix this, see
DB Config Page and Installer Project #2380

Also, notice the Ten Core Constants this Retires (eliminates), and how they are replaced in the DB ($ydb) at the bottom of this working demo page...
http://a.fil.net/s/admin/db-config.php?dev=notes

  • YOURLS_SITE is eliminated and so is the trailing slash problem.

Core Constants eliminated include:

  1. YOURLS_HOURS_OFFSET (eliminated DST/Summertime offset problem)
  2. YOURLS_LANG
  3. YOURLS_UNIQUE_URLS
  4. YOURLS_PRIVATE
  5. YOURLS_COOKIEKEY
  6. YOURLS_URL_CONVERT
  7. YOURLS_DEBUG
  8. $yourls_reserved_URL (array)
  9. YOURLS_ADMIN_SSL (https code bloat)
  10. YOURLS_SITE (trailing slash problem)

Constants are a problem for many plugins under developement.

@PopVeKind

This comment has been minimized.

Show comment
Hide comment
@PopVeKind

PopVeKind Apr 19, 2018

Contributor

The HTTPS Plugin (and PR) will totally eliminate the trailing slash error. #2391 (comment)

Contributor

PopVeKind commented Apr 19, 2018

The HTTPS Plugin (and PR) will totally eliminate the trailing slash error. #2391 (comment)

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