Обсуждение: Patch for RM1911 Direct file navigation [pgAdmin4] [Feature]

Hi

On Sat, Nov 19, 2016 at 5:57 PM, Harshal Dhumal
<harshal.dhumal@enterprisedb.com> wrote:
> Hi,
>
> PFA for RM1911
>
> Feature added: Allow user add file/folder path directly in input field.
> Other improvements like clean up after file manager is closed.

I did a quick check on Mac, in server mode and immediately hit problems:

- [Screenshot 1]: I typed in "/Foo2" to navigate into the Foo2
directory. a) it didn't navigate as expected, and b) it gave me a very
non-user friendly error message.

- [Screenshot 2]: I typed in "foo2.sql". It added the leading /
automatically, then gave the error dpagefoo2.sql does not exist.

Then on Windows in desktop mode:

- [Screenshot 3]: I typed in "C:\Users\dpage" (which does exist) and
got the error shown.

- [Screenshot 4]: I typed in "\\172.16.253.235\data" (which does
exist) and got the error shown.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi

On Thu, Nov 24, 2016 at 10:25 AM, Harshal Dhumal
<harshal.dhumal@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find updated attached patch for RM1911 (V2)
>
> --
> Harshal Dhumal
> Software Engineer
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, Nov 21, 2016 at 7:48 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Sat, Nov 19, 2016 at 5:57 PM, Harshal Dhumal
>> <harshal.dhumal@enterprisedb.com> wrote:
>> > Hi,
>> >
>> > PFA for RM1911
>> >
>> > Feature added: Allow user add file/folder path directly in input field.
>> > Other improvements like clean up after file manager is closed.
>>
>> I did a quick check on Mac, in server mode and immediately hit problems:
>>
>> - [Screenshot 1]: I typed in "/Foo2" to navigate into the Foo2
>> directory. a) it didn't navigate as expected, and b) it gave me a very
>> non-user friendly error message.
>>
> Fixed.
>>
>> - [Screenshot 2]: I typed in "foo2.sql". It added the leading /
>> automatically, then gave the error dpagefoo2.sql does not exist.
>>
> Fixed.
>>
>> Then on Windows in desktop mode:
>>
>> - [Screenshot 3]: I typed in "C:\Users\dpage" (which does exist) and
>> got the error shown.
>>
> Fixed.
>>
>> - [Screenshot 4]: I typed in "\\172.16.253.235\data" (which does
>> exist) and got the error shown.
>
> Fixed.

- If I type in a path and hit return, it's properly opened. If I hit
the Refresh button, it reloads a different location - in my case, for
/Users/dpage it alwoys goes back to /Users. Oddly, it doesn't do that
in /usr/bin.

- The issue above is also seen with /Test_Folder (when running in
server mode). It refreshes back to /

- If I navigate to /abc/123/, then upload a file called emails.txt, it
gets saved as /abc/123emails.txt.

- On Windows (in server mode), if I try to navigate to
\\vmware-host\Shared Folders\dpage (which exists and is accessible
from the machine/user), it changes the name to "/\\vmware-host\Shared
Folders\dpage" and gives an error that "dpage" doesn't exist.

- After many of these errors, the OK button is enabled, and clicking
it gives an error: "Invalid mode ('rb') or filename"

- On Windows (in desktop mode), I'm stuck in the root of the C drive.
I cannot click the Up directory button to see the available drives.

- On Windows (in desktop mode), if I type "C:\" into the location bar,
it navigates to the folder, but changes the path to "C:\/"

- On windows (in desktop mode), if I type "\\172.16.253.235\data" into
the location bar, I get "'' file does not exist"

Please test thoroughly on Windows and *nix, in both server and desktop
mode before resubmitting.

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi

On Fri, Dec 16, 2016 at 6:46 PM, Harshal Dhumal
<harshal.dhumal@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find updated patch below for direct file navigation.
> I have covered all of above mentioned case.

Still not there I'm afraid:

- On Mac, if I type \Users\dpage, it changes it to /\Users\dpage and
then tells me it doesn't exist. Per the RM, either forward or back
slashes should be acceptable ("The path should accept either / or \ as
separators. Upon successful navigation to the path (after pressing
Return), the slashes should be replaced with the platform standard if
needed.")

- Unicode handling seems to be completely broken - see the attached screenshot.

I haven't tested on Windows yet, and only in Desktop mode on Mac.
Please test on Windows and Mac or Linux with both Python 2 and 3, in
both Server and Desktop modes to ensure that the behaviour meets the
requirements of the ticket with Unicode and non-Unicode paths and
files before resubmitting.

Thanks.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

Hi

On Sat, Jan 14, 2017 at 2:27 PM, Harshal Dhumal
<harshal.dhumal@enterprisedb.com> wrote:
> Hi,
>
> Pls updated patch for RM1911.
>
> 1. This includes fix for issue index out of range when user enters path of
> folder without trailing slash (showed by Dave).
> 2. To make this functionality compatible with save last used directory
> feature.

- The first test I ran gave the error seen in the attachment (running
in server mode, clicking the Browse button on the backup dialogue).

- I also noticed in reviewing the changes again, that you've got code
in sqleditor/__init__.py to stop the user moving outside of the
storage sandbox in server mode. That code should be part of the file
manager - none of the modules using it should be doing that kind of
check.

- If I do try to navigate outside of the sandbox, I get a nice error:
"Error: Access Denied (/Users/dpage/.pgadmin)" for example, if I enter
/../../. Whilst it's good to be informative, it's also a security
leak. It should only tell me the path that the user sees, not the path
as it actually is on the server - e.g.  "Error: Access Denied
(/../../../)"

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers