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

ensure we properly escape such that special characters will work #6

Merged
merged 2 commits into from
Jan 10, 2022

Conversation

mgallien
Copy link
Contributor

@mgallien mgallien commented Jan 7, 2022

Signed-off-by: Matthieu Gallien [email protected]

@mgallien mgallien requested a review from artonge January 7, 2022 11:08
@@ -34,7 +34,8 @@ function correct_mtime() {
username=$(dirname "$username")
done

relative_filepath_without_username="${relative_filepath/#$username\//}"
first_relative_filepath_without_username="${relative_filepath/#$username\//}"
relative_filepath_without_username=`echo $first_relative_filepath_without_username | sed "s/\"/\\\\\\\\\"/g"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't get the sed command

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 is about escaping double quotes such that when we execute the sql command via command line they are not confused with the delimitors
this is to handle the extreme case where double quotes would happen inside the file name

Copy link
Collaborator

@artonge artonge Jan 7, 2022

Choose a reason for hiding this comment

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

But why so many \ ?

s/\"/\\\\\\\\\"/g would replace " by \\\\", no?

And lets use proper naming:

first_relative_filepath_without_username => relative_filepath_without_username
relative_filepath_without_username => relative_filepath_without_username_escaped

Suggested change
relative_filepath_without_username=`echo $first_relative_filepath_without_username | sed "s/\"/\\\\\\\\\"/g"`
relative_filepath_without_username="${relative_filepath/#$username\//}"
relative_filepath_without_username_escaped=`echo $relative_filepath_without_username | sed "s/\"/\\\\\\\\\"/g"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can only say that without that many \ it was not working
my understanding is that you get those three times through different shells and thus we need that many of them
I will fix the naming

Copy link
Collaborator

@artonge artonge Jan 7, 2022

Choose a reason for hiding this comment

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

I remember testing a filename with a quote in it and it was working fine, so I am not sure it is necessary to do that. I will retest on Monday.

Copy link
Collaborator

Choose a reason for hiding this comment

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

An error with single quotes seem more likely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes this is the reason to use double quotes to escape the string when calling mysql or psql

Copy link
Collaborator

Choose a reason for hiding this comment

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

Using base64 might be safer:

INSERT INTO active_records (password) VALUES (FROM_BASE64('$(echo -n $PASSWORD|base64)'))

From: https://stackoverflow.com/questions/4383135/escaping-mysql-command-lines-via-bash-scripting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah why not
it took me a while to understand the idea and yes it looks like it should be less hacky

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@artonge I have applied the base64 solution

@mgallien mgallien force-pushed the handleSpecialCharacters branch from acbaa52 to 8cdbb08 Compare January 7, 2022 14:50
@X-dark
Copy link

X-dark commented Jan 10, 2022

Latest patch version is not working with pgsql. Filenames are interpreted as column name instead of value.

@X-dark
Copy link

X-dark commented Jan 10, 2022

For postgresql, sed escaping is unnecessary. What works for me is:

mtime_in_db=$(
  psql \
    "postgresql://$db_user:$db_pwd@$db_host/$db_table" \
    --tuples-only \                                                                                                                                                     
    --no-align \
    -E 'UTF-8' \
    --command="\
      SELECT mtime
      FROM oc_storages JOIN oc_filecache ON oc_storages.numeric_id = oc_filecache.storage \
      WHERE oc_storages.id='home::$username' AND oc_filecache.path=\$\$${relative_filepath_without_username}\$\$"
)

This relies on Dollar-Quoted Strings constants: https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-CONSTANTS

@mgallien mgallien force-pushed the handleSpecialCharacters branch from 8cdbb08 to a0b4c5a Compare January 10, 2022 14:33
@mgallien
Copy link
Contributor Author

For postgresql, sed escaping is unnecessary. What works for me is:

mtime_in_db=$(
  psql \
    "postgresql://$db_user:$db_pwd@$db_host/$db_table" \
    --tuples-only \                                                                                                                                                     
    --no-align \
    -E 'UTF-8' \
    --command="\
      SELECT mtime
      FROM oc_storages JOIN oc_filecache ON oc_storages.numeric_id = oc_filecache.storage \
      WHERE oc_storages.id='home::$username' AND oc_filecache.path=\$\$${relative_filepath_without_username}\$\$"
)

This relies on Dollar-Quoted Strings constants: https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-CONSTANTS

thanks @X-dark (by the way one of my former colleague is sharing your full name, how funny !)

+ Fix trailing line return in base64 encoding

Signed-off-by: Louis Chemineau <[email protected]>
@artonge
Copy link
Collaborator

artonge commented Jan 10, 2022

Adapted the solution for pgsql, and fixed a problem with trailing line return added in the encoded value.

Tested with filename containing ', ", (, and ;.

@artonge artonge merged commit bc73f68 into master Jan 10, 2022
@mgallien mgallien deleted the handleSpecialCharacters branch January 12, 2022 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants