Escape characters in file path #508

Closed
bbimber opened this Issue Aug 27, 2014 · 7 comments

Comments

Projects
None yet
2 participants
@bbimber

bbimber commented Aug 27, 2014

I tried to execute a command like the following:

[labkey@prime-seq ~]$ perl /var/www/html/jbrowse/bin/flatfile-to-json.pl --gff "/home/groups/prime-seq/production/Shared/@files/.referenceLibraries/UNMC Annotation.gff" --trackLabel "UNMC Annotation" --key "3 UNMC Annotation" --compress --out "/home/groups/prime-seq/production/Shared/@files/.jbrowse/tracks/3"

note the @ in the filepath. it gives an error like:

Possible unintended interpolation of @files in string at (eval 27) line 1.
Use of uninitialized value in subroutine entry at /var/www/html/jbrowse/bin/../src/perl5/NameHandler.pm line 88.
Can't use string ("") as a subroutine ref while "strict refs" in use at /var/www/html/jbrowse/bin/../src/perl5/NameHandler.pm line 88.

It seems like either:

  1. the scripts should support escapes within the filepath. i tried this, but then the script blew up b/c it wasnt able to resolve the file.

  2. the script should do some internal encoding of the strings.

@cmdcolin

This comment has been minimized.

Show comment
Hide comment
@cmdcolin

cmdcolin Aug 29, 2014

Contributor

I can definitely confirm that the script behaves problematically in this case. I don't have a workaround at the moment but if you have any fixes let us know!

Contributor

cmdcolin commented Aug 29, 2014

I can definitely confirm that the script behaves problematically in this case. I don't have a workaround at the moment but if you have any fixes let us know!

@bbimber

This comment has been minimized.

Show comment
Hide comment
@bbimber

bbimber Sep 12, 2014

Hi Colin,

Would you consider adding a dependency on File::Util to the scripts?

http://search.cpan.org/~tommy/File-Util-3.27/Util.pod

escape_filename() is the best option I can find for standardized escaping
of special characters.

-Ben

On Fri, Aug 29, 2014 at 2:45 PM, Colin notifications@github.com wrote:

I can definitely confirm that the script behaves problematically in this
case. I don't have a workaround at the moment but if you have any fixes let
us know!


Reply to this email directly or view it on GitHub
#508 (comment).

bbimber commented Sep 12, 2014

Hi Colin,

Would you consider adding a dependency on File::Util to the scripts?

http://search.cpan.org/~tommy/File-Util-3.27/Util.pod

escape_filename() is the best option I can find for standardized escaping
of special characters.

-Ben

On Fri, Aug 29, 2014 at 2:45 PM, Colin notifications@github.com wrote:

I can definitely confirm that the script behaves problematically in this
case. I don't have a workaround at the moment but if you have any fixes let
us know!


Reply to this email directly or view it on GitHub
#508 (comment).

@bbimber

This comment has been minimized.

Show comment
Hide comment
@bbimber

bbimber Sep 12, 2014

Actually I'm wrong on the right perl helper, but the general question is
the same: are you OK with potentially adding a dependency in order to bring
in another perl module that does a standardized file normalization?

-Ben

On Fri, Sep 12, 2014 at 8:29 AM, Ben Bimber bbimber@gmail.com wrote:

Hi Colin,

Would you consider adding a dependency on File::Util to the scripts?

http://search.cpan.org/~tommy/File-Util-3.27/Util.pod

escape_filename() is the best option I can find for standardized escaping
of special characters.

-Ben

On Fri, Aug 29, 2014 at 2:45 PM, Colin notifications@github.com wrote:

I can definitely confirm that the script behaves problematically in this
case. I don't have a workaround at the moment but if you have any fixes let
us know!


Reply to this email directly or view it on GitHub
#508 (comment).

bbimber commented Sep 12, 2014

Actually I'm wrong on the right perl helper, but the general question is
the same: are you OK with potentially adding a dependency in order to bring
in another perl module that does a standardized file normalization?

-Ben

On Fri, Sep 12, 2014 at 8:29 AM, Ben Bimber bbimber@gmail.com wrote:

Hi Colin,

Would you consider adding a dependency on File::Util to the scripts?

http://search.cpan.org/~tommy/File-Util-3.27/Util.pod

escape_filename() is the best option I can find for standardized escaping
of special characters.

-Ben

On Fri, Aug 29, 2014 at 2:45 PM, Colin notifications@github.com wrote:

I can definitely confirm that the script behaves problematically in this
case. I don't have a workaround at the moment but if you have any fixes let
us know!


Reply to this email directly or view it on GitHub
#508 (comment).

@cmdcolin

This comment has been minimized.

Show comment
Hide comment
@cmdcolin

cmdcolin Sep 12, 2014

Contributor

@bbimber: Sure I would definitely consider that.
One thing I did notice though is that this error only occurs when the --out parameter has special characters, and not necessarily the regular --gff parameter to flatfile-to-json.

Maybe that can help track down where to create a bugfix!

Contributor

cmdcolin commented Sep 12, 2014

@bbimber: Sure I would definitely consider that.
One thing I did notice though is that this error only occurs when the --out parameter has special characters, and not necessarily the regular --gff parameter to flatfile-to-json.

Maybe that can help track down where to create a bugfix!

@cmdcolin

This comment has been minimized.

Show comment
Hide comment
@cmdcolin

cmdcolin Sep 12, 2014

Contributor

Hi Ben,
We were going over a similar bug in WebApollo today and found out that single quotes might be the way to handle things, so this patch seems to fix this bug.

diff --git a/src/perl5/FeatureTrack.pm b/src/perl5/FeatureTrack.pm
index 1f67c75..333be6f 100644
--- a/src/perl5/FeatureTrack.pm
+++ b/src/perl5/FeatureTrack.pm
@@ -148,7 +148,7 @@ sub nameHandler { $_[0]->{nameHandler} }
 sub _make_nameHandler {
     my ( $self ) = @_;
     (my $trackdir = $self->{trackDirTemplate}) =~ s/\{refseq\}/'$_[0]'/eg;
-    $self->{nameHandler} = NameHandler->new( eval qq|sub { "$trackdir" }| );
+    $self->{nameHandler} = NameHandler->new( eval qq|sub { '$trackdir' }| );
 }

Extra note: loading files via the ?data=@files via the URL already works by default too

Contributor

cmdcolin commented Sep 12, 2014

Hi Ben,
We were going over a similar bug in WebApollo today and found out that single quotes might be the way to handle things, so this patch seems to fix this bug.

diff --git a/src/perl5/FeatureTrack.pm b/src/perl5/FeatureTrack.pm
index 1f67c75..333be6f 100644
--- a/src/perl5/FeatureTrack.pm
+++ b/src/perl5/FeatureTrack.pm
@@ -148,7 +148,7 @@ sub nameHandler { $_[0]->{nameHandler} }
 sub _make_nameHandler {
     my ( $self ) = @_;
     (my $trackdir = $self->{trackDirTemplate}) =~ s/\{refseq\}/'$_[0]'/eg;
-    $self->{nameHandler} = NameHandler->new( eval qq|sub { "$trackdir" }| );
+    $self->{nameHandler} = NameHandler->new( eval qq|sub { '$trackdir' }| );
 }

Extra note: loading files via the ?data=@files via the URL already works by default too

@cmdcolin cmdcolin closed this in 10fff61 Sep 12, 2014

cmdcolin added a commit that referenced this issue Sep 13, 2014

@cmdcolin

This comment has been minimized.

Show comment
Hide comment
@cmdcolin

cmdcolin Sep 16, 2014

Contributor

Apparently I broke some perl tests with my proposed bugfix here. Details here https://travis-ci.org/cmdcolin/jbrowse/builds/35389757

Contributor

cmdcolin commented Sep 16, 2014

Apparently I broke some perl tests with my proposed bugfix here. Details here https://travis-ci.org/cmdcolin/jbrowse/builds/35389757

@cmdcolin

This comment has been minimized.

Show comment
Hide comment
@cmdcolin

cmdcolin Sep 16, 2014

Contributor

I double checked this fix because I was a little worried about how quotemeta would behave but this now seems to work with the following tests.

International characters (Chinese unicode)
Dollar signs ($)
At symbol (@)

Unicode worked before this patch too but I just wanted to make sure that it also worked after the patch too.

Contributor

cmdcolin commented Sep 16, 2014

I double checked this fix because I was a little worried about how quotemeta would behave but this now seems to work with the following tests.

International characters (Chinese unicode)
Dollar signs ($)
At symbol (@)

Unicode worked before this patch too but I just wanted to make sure that it also worked after the patch too.

@cmdcolin cmdcolin added this to the 1.11.6 milestone Jan 23, 2015

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