Fix sed instructions relative to file .htaccess

[colisee]

Close issue #198

[ikke-t]

I need to check apache docs to understand the format first. I don't get it from here what difference this makes. But my first question is why this is needed in container only? Why is this not fixed in original librebooking git repo .htaccess file?

[JohnVillalovos]

Unsure if I am doing anything wrong but I took the sed snippet and applied it it .htaccess and I don't see a difference between the old and new versions

#!/bin/bash

set -u
# set -e

export PS4='+ ${BASH_SOURCE:-}:${FUNCNAME[0]:-}:L${LINENO:-}:   '
# set -x

APP_PATH=FOO

# old
cp .htaccess .htaccess-old
sed \
 -i .htaccess-old \
    -e "s:\(RewriteCond .*\)/Web:\1/${APP_PATH}/Web:" \
    -e "s:\(RewriteRule .*\)/Web:\1/${APP_PATH}/Web:"

cp .htaccess .htaccess-new
sed \
    -i .htaccess-new \
    -e "s:\(RewriteCond .*\^\)/Web:\1/${APP_PATH}/Web:" \
    -e "s:\(RewriteRule .*\) /Web:\1 /${APP_PATH}/Web:"

diff -u .htaccess-old .htaccess-new
echo $?

[ikke-t]

I'm on travel, and only on mobile. So I'm likely wrong, but to me it looks like the sed removes the whole "RewriteRule" and "...Cond" as it's within the '()'. Check if it isn't so.

[ikke-t]

I mean this \(RewriteCond should likely be RewriteCond \(

[colisee]

I need to check apache docs to understand the format first. I don't get it from here what difference this makes. But my first question is why this is needed in container only? Why is this not fixed in original librebooking git repo .htaccess file?

@ikke-t , it is related to the docker image when used with the docker environment variable APP_PATH. If you restart a running container, the entrypoint.sh script will modify the /var/www/html/.htaccess file although it shouldn't. The reason being the incorrect sed statement.

[colisee]

Unsure if I am doing anything wrong but I took the sed snippet and applied it it .htaccess and I don't see a difference between the old and new versions

#!/bin/bash

set -u
# set -e

export PS4='+ ${BASH_SOURCE:-}:${FUNCNAME[0]:-}:L${LINENO:-}:   '
# set -x

APP_PATH=FOO

# old
cp .htaccess .htaccess-old
sed \
 -i .htaccess-old \
    -e "s:\(RewriteCond .*\)/Web:\1/${APP_PATH}/Web:" \
    -e "s:\(RewriteRule .*\)/Web:\1/${APP_PATH}/Web:"

cp .htaccess .htaccess-new
sed \
    -i .htaccess-new \
    -e "s:\(RewriteCond .*\^\)/Web:\1/${APP_PATH}/Web:" \
    -e "s:\(RewriteRule .*\) /Web:\1 /${APP_PATH}/Web:"

diff -u .htaccess-old .htaccess-new
echo $?

You will see the difference when you restart an existing container.

I reckon that the .htaccess-related sed statement is quite "tricky"... Does Copilot offer a simpler/more readable solution?

[ikke-t]

Like said, I claim it's wrong. It should not substitute the key word Rewrite...

[ikke-t]

Playing in my mobile while waiting for ferry. I find this good to read and robust:

u0_a466@localhost  ~  cat htaccess                   RewriteEngine On                                        # Set RewriteBase if your physical path is different from the URL. For example, if using an alias

# Uncomment the following two lines to force HTTPS
#RewriteCond %{HTTPS} off
#RewriteRule .* https://%{HTTP_HOST}%{REQUEST_URI} [L,R=301]

# Redirect requests that are not already under /Web to /Web.
RewriteCond %{REQUEST_URI} !^/Web(/|$)
RewriteRule ^(.*)$ /Web/$1 [R=301,L]

#Header Set Access-Control-Allow-Origin "*"
#Header add Access-Control-Allow-Headers "origin, x-requested-with, content-type"
#Header add Access-Control-Allow-Methods "PUT, GET, POST, DELETE, OPTIONS"
#php_value max_input_vars 10000

 u0_a466@localhost  ~  sed -e "s:\(RewriteCond.*\^\)\(/Web.*\):\1/${APP_PATH}\2:"  -e "s:\(RewriteRule.*\$ \)\(/Web.*\):\1/${APP_PATH}\2:" htaccess
RewriteEngine On
# Set RewriteBase if your physical path is different from the URL. For example, if using an alias

# Uncomment the following two lines to force HTTPS
#RewriteCond %{HTTPS} off
#RewriteRule .* https://%{HTTP_HOST}%{REQUEST_URI} [L,R=301]

# Redirect requests that are not already under /Web to /Web.
RewriteCond %{REQUEST_URI} !^/foo/Web(/|$)
RewriteRule ^(.*)$ /foo/Web/$1 [R=301,L]

#Header Set Access-Control-Allow-Origin "*"
#Header add Access-Control-Allow-Headers "origin, x-requested-with, content-type"
#Header add Access-Control-Allow-Methods "PUT, GET, POST, DELETE, OPTIONS"
#php_value max_input_vars 10000

[ikke-t]

Actually, this is the neatest and the most readable:

sed -e "/^Rewrite/s:Web:${APP_PATH}/Web:"

It finds rows starting with Rewrite, and substitutes Web to ${APP_PATH}/Web on matching lines.

Much simpler to read.

This would not work if run several times to .htaccess, but likely it's not.

[colisee]

Actually, this is the neatest and the most readable:

sed -e "/^Rewrite/s:Web:${APP_PATH}/Web:"

It finds rows starting with Rewrite, and substitutes Web to ${APP_PATH}/Web on matching lines.

Much simpler to read.

This would not work if run several times to .htaccess, but likely it's not.

I find the /^Rewrite/ very clever. But unfortunately, the sed statement is buggy.
If you run it twice (as if you were simulating a container restart or a container stop & start), then you end up with the incorrect RewriteCond and RewriteRule.

You can see for yourself:

1. Set the original/upstream .htaccessfile

cat <<EOF | tee .htaccess
RewriteEngine On
# Set RewriteBase if your physical path is different from the URL. For example, if using an alias

# Uncomment the following two lines to force HTTPS
#RewriteCond %{HTTPS} off
#RewriteRule .* https://%{HTTP_HOST}%{REQUEST_URI} [L,R=301]

# Redirect requests that are not already under /Web to /Web.
RewriteCond %{REQUEST_URI} !^/Web(/|$)
RewriteRule ^(.*)$ /Web/$1 [R=301,L]

#Header Set Access-Control-Allow-Origin "*"
#Header add Access-Control-Allow-Headers "origin, x-requested-with, content-type"
#Header add Access-Control-Allow-Methods "PUT, GET, POST, DELETE, OPTIONS"
#php_value max_input_vars 10000
EOF

2. Update .htaccess the first time (this simulates the container being created and ran)

APP_PATH=foo
sed  -i .htaccess -e "/^Rewrite/s:Web:${APP_PATH}/Web:"

3. Update .htaccess a second time (this simulates the container being restarted or stopped and started)

sed  -i .htaccess -e "/^Rewrite/s:Web:${APP_PATH}/Web:"

4 Look at the content of .htaccess. You end up with APP_PATH being doubled:

RewriteCond %{REQUEST_URI} !^/foo/foo/Web(/|$)
RewriteRule ^(.*)$ /foo/foo/Web/ [R=301,L]

[ikke-t]

OK, I was thinking we do that only at container creation time. This fixed it, if it's already applied we skip it:

sed "/^Rewrite/ { /${APP_PATH}\/Web/! s:Web:${APP_PATH}/Web: }"

[JohnVillalovos]

You will see the difference when you restart an existing container.

I reckon that the .htaccess-related sed statement is quite "tricky"... Does Copilot offer a simpler/more readable solution?

Ah. Okay.

What if do something like.

if [[ ! -e .htaccess.orig ]]; then
   # make a copy of the original `.htaccess` file
    cp -a .htaccess .htaccess.orig
fi
# use the original copy each time as this runs every time a container is restarted
# and the `sed` command is not idempotent
cp -a .htaccess.orig .htaccess

# Do the `sed` command here

Instead of trying to make a sed command that can be idempotent. Which seems difficult.

[ikke-t]

sounds good. The last sed should work though, at least on command line it behaves right. Only time it would not work is if somehow the APP_PATH would change between the invocations, but I don't see it changing during the container lifecycle.

[colisee]

@ikke-t : Thank you for the last sed instruction. It works and I learnt something today 👍

@JohnVillalovos : Your suggestion to implement an idempotent code makes complete sense and I will implement this. Once this PR is updated, either you or @ikke-t can approve it and confirm the deletion of the remote issues/198 branch