FWD » Core Development » User Interface

Some use cases in 4GL code are reading local files (on a user's desktop system) based on the selections made in SYSTEM-DIALOG GET-FILE (and possibly in SYSTEM-DIALOG GET-DIR).

The idea in this task is to add an optional 4GL keyword (as a FWD-specific enhancement) to notify the FWD runtime that instead of using the standard SYSTEM-DIALOG GET-FILE/SYSTEM-DIALOG GET-DIR dialogs, we should implement using the browser's file dialog and upload the selected files to a specific location on the FWD client system. The 4GL code could then access these files as if they were local. The browser's dialog MUST be used otherwise upload is not possible.

This will be a bit tricky as the browser will only allow JS to access file data that the user actually selects, and the selection must happen in the handler of an event the user initiates. Opening browse file upload dialog from the legacy code alone won't work. A possible workaround could be a window/iframe popup with <input type="file" .../> (or other elements) with a handler that would take the selected files and upload them o the server, then close the popup.

A possible workaround could be a window/iframe popup with <input type="file" .../> (or other elements) with a handler that would take the selected files and upload them o the server, then close the popup.

Yes, this is the exact idea. We already do this in some browsers for copying data to the system clipboard.

As an additional feature, please also add support for multiple selection (optionally returning a list of multiple files selected by the user).

I propose using the MULTIPLE keyword. It already exists and is used for this same purpose in both BROWSE and SELECTION-LIST.

Another feature is to allow browser file "selection" by the user but without upload. This might be used as a way to store just the filenames/directory names without the data, which is a use case seen in some applications. Perhaps this is an AT-WEB-BROWSER option. Use of UPLOAD would be implicitly also done a the web browser but would also upload the data.

I wonder, if one would use SYSTEM-DIALOG GET-FILE mycharactervariable AT-WEB-BROWSER UPLOAD, and the end-user selects "c:\foo\bar.txt" and this file gets automatically uploaded to the server to, for example, "/v1/fwd/tmp/uploads/foo/bar.txt" then what is the value of mycharactervariabe? Both "c:\foo\bar.txt" and "/v1/fwd/tmp/uploads/foo/bar.txt" are useful values.

Jurjen Dijkstra wrote:

I wonder, if one would use SYSTEM-DIALOG GET-FILE mycharactervariable AT-WEB-BROWSER UPLOAD, and the end-user selects "c:\foo\bar.txt" and this file gets automatically uploaded to the server to, for example, "/v1/fwd/tmp/uploads/foo/bar.txt" then what is the value of mycharactervariabe? Both "c:\foo\bar.txt" and "/v1/fwd/tmp/uploads/foo/bar.txt" are useful values.

I had initially thought that both would not be needed at the same time. But if both are needed, then we would need another variable to capture the "remote" name.

I think mycharactervariable should probably report "/v1/fwd/tmp/uploads/foo/bar.txt" so that code that uses it could be unchanged. The idea is that when possible it should report a filename that is visible to the calling 4GL code.

If the original "c:\foo\bar.txt" name is of interest, we could provide AT-WEB-BROWSER mycharremotenamevar to capture that name. If UPLOAD is not specified, then no mycharremotenamevar would be needed because the remote name is the only one available so we would put the remote name in mycharactervariable.

The other thought here is that we probably should provide UPLOAD <char_expression_with_upload_dir_name> which in your example would be set to UPLOAD "/v1/fwd/tmp/uploads/".

Greg Shah wrote:

I had initially thought that both would not be needed at the same time. But if both are needed, then we would need another variable to capture the "remote" name.

Yes I know of at least one use-case where the original filename is required because it has to be stored. Oh, that reminds me, that original filename is also used as the default value for the SYSTEM-DIALOG GET-FILE SAVE-AS statement when the end-user (or a different end-user) downloads the file later.

The other thought here is that we probably should provide UPLOAD <char_expression_with_upload_dir_name> which in your example would be set to UPLOAD "/v1/fwd/tmp/uploads/".

Yes I thought so too.
That value does not make a roundtrip to the browser I hope? Just to prevent that someone who is creative with the Chrome debugger can change it to upload "/etc/passwd" :-)

That value does not make a roundtrip to the browser I hope? Just to prevent that someone who is creative with the Chrome debugger can change it to upload "/etc/passwd" :-)

Right. There is no need for it to be sent to the browser.

In many cases (including this one), we add specific keywords/options to FWD to enhance existing language statements or other 4GL features. These new features are specific to FWD and are not naturally backward compatible with the 4GL. To enable customers to maintain a single set of application sources, we have provided the FWD-VERSION preprocessor definition. This is very useful but it tends to be a multi-line construct.

Customers have asked for an alternative that is more condensed. Jurjen and others have suggested the idea that we create built-in preprocessor definitions for more of our new keywords. For example, we could add a FWD-AT-WEB-BROWSER definition that exists by default in the FWD preprocessor and the value would be AT-WEB-BROWSER. Customers could encode things like this:

SYSTEM-DIALOG GET-FILE mycharactervariable {&FWD-AT-WEB-BROWSER}.

In OpenEdge it would be silently dropped but in FWD it would have the expected meaning. This is a much more condensed result. Limitations:

• We would have to maintain a potentially large set of these over time.
• Some use cases don't work cleanly. For example, in our UPLOAD <directory-name-char-expression> there is no way to handle this with only a simple preprocessor definition. The 4GL has no equivalent to C/C++ preprocessor macros.

Anyway, we should consider this idea as part of this work.

Sergey/Constantin: Please do a code review.

Hynek, could you explain more thoroughly how to reproduce OutOfMemoryException in the web gui driver process. Did you have a standalone test? I didn't observe OutOfMemoryException exceptions when testing MSG_PARTIAL with large file.

Sergey Ivanovskiy wrote:

Hynek, could you explain more thoroughly how to reproduce OutOfMemoryException in the web gui driver process. Did you have a standalone test? I didn't observe OutOfMemoryException exceptions when testing MSG_PARTIAL with large file.

What was the max size you tried? I was getting it for sizes around several hundreds of megabytes.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek, could you explain more thoroughly how to reproduce OutOfMemoryException in the web gui driver process. Did you have a standalone test? I didn't observe OutOfMemoryException exceptions when testing MSG_PARTIAL with large file.

What was the max size you tried? I was getting it for sizes around several hundreds of megabytes.

Yes, I should find the development branch but files having 500Mb to 1Gb should be tested. When I tested MSG_PARTIAL implementation there were no GUI for file uploading.
Please provide the test cases on which this issue can be reproduced.

OK. MSG_FILE_UPLOADING is implemented correctly and large files can be uploaded. But message MSG_FILE_CHOOSE is implemented differently. Could you use the same idea as MSG_FILE_UPLOADING uses? Is it possible for your use case?

Sergey Ivanovskiy wrote:

OK. MSG_FILE_UPLOADING is implemented correctly and large files can be uploaded. But message MSG_FILE_CHOOSE is implemented differently. Could you use the same idea as MSG_FILE_UPLOADING uses? Is it possible for your use case?

If I understand correctly the code, then this method protected boolean processChannel(FileChannel channel) of GuiWebSocket should work for MSG_FILE_UPLOADING.

Sergey Ivanovskiy wrote:

Sergey Ivanovskiy wrote:

OK. MSG_FILE_UPLOADING is implemented correctly and large files can be uploaded. But message MSG_FILE_CHOOSE is implemented differently. Could you use the same idea as MSG_FILE_UPLOADING uses? Is it possible for your use case?

If I understand correctly the code, then this method protected boolean processChannel(FileChannel channel) of GuiWebSocket should work for MSG_FILE_UPLOADING.

Yes, and processBinaryMessage is invoked only for small files.

Sergey Ivanovskiy wrote:

OK. MSG_FILE_UPLOADING is implemented correctly and large files can be uploaded. But message MSG_FILE_CHOOSE is implemented differently. Could you use the same idea as MSG_FILE_UPLOADING uses? Is it possible for your use case?

I was considering to use the partial web socket protocol but then decided not to use it for the following reasons.

1. The partial message web socket protocol has some direct references to drag&drop. Not big deal, but some changes would be needed to make it a bit more generic to cover file uploads (multiple files, Java identifiers referencing drag&drop, etc.).
2. The protocol creates two temporary copies of the uploaded messages. So for uploading multiple files to a specific directory there would be needed 3 times the space of the uploaded data on the file system.
3. I used the dojo upload UI which out of the box worked with HTML5 uploads.

I think that improving on Jetty's upload handling would be less work than using the partial web socket protocol to resolve the memory issue.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

OK. MSG_FILE_UPLOADING is implemented correctly and large files can be uploaded. But message MSG_FILE_CHOOSE is implemented differently. Could you use the same idea as MSG_FILE_UPLOADING uses? Is it possible for your use case?

I was considering to use the partial web socket protocol but then decided not to use it for the following reasons.

1. The partial message web socket protocol has some direct references to drag&drop. Not big deal, but some changes would be needed to make it a bit more generic to cover file uploads (multiple files, Java identifiers referencing drag&drop, etc.).

I would like to note that MSG_PARTIAL is used to send a message via a web socket if this message size exceeds the maximal message size excepted by a web socket and web socket api has no stream api at this moment. Thus file channels to temporary files can help to use data streams.

2. The protocol creates two temporary copies of the uploaded messages. So for uploading multiple files to a specific directory there would be needed 3 times the space of the uploaded data on the file system.

There is only one temporary file per a message.

Sergey Ivanovskiy wrote:

2. The protocol creates two temporary copies of the uploaded messages. So for uploading multiple files to a specific directory there would be needed 3 times the space of the uploaded data on the file system.

There is only one temporary file per a message.

See the call to Files.createTempFile in WCP.processPartialMesssage and the call this.callbacks.saveDropTargetFile in GWS.processChannel(). Unless I interpret the code wrong, there are two temp files created during d&d file upload.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

2. The protocol creates two temporary copies of the uploaded messages. So for uploading multiple files to a specific directory there would be needed 3 times the space of the uploaded data on the file system.

There is only one temporary file per a message.

See the call to Files.createTempFile in WCP.processPartialMesssage and the call this.callbacks.saveDropTargetFile in GWS.processChannel(). Unless I interpret the code wrong, there are two temp files created during d&d file upload.

Yes, you are correct but it can be fixed by moving one file to another. I tried to support only the conception to use one temporary file per a large split message.

Is there a help file or wiki page for SYSTEM-DIALOG GET-FILE with the new options?

I have read comment #12 but it leaves room for interpretation, and for questions. Like: is myremotecharactervariable an input, output or input-output parameter. Can DIR "/tmp" be replaced by DIR mydirectoryvariable or should it be DIR VALUE. Are there any limitations on the value of mydirectoryvariable can be, like can it be SESSION:TEMP-DIR or does it have to be a (sub)directory of the upload directory of the Jetty server and if so how can we get that path from 4GL in FWD. What does "MULTIPLE option currently only works for the web uploader" mean, what is a web uploader?

A Help file, possibly with one or two example snippets, would be much appreciated.

Jurjen Dijkstra wrote:

Is there a help file or wiki page for SYSTEM-DIALOG GET-FILE with the new options?

I will create the documentation when the code changes are reviewed.

I have read comment #12 but it leaves room for interpretation, and for questions. Like: is myremotecharactervariable an input, output or input-output parameter.

myremotecharactervariable is only assigned, it behaves like an output parameter.

Can DIR "/tmp" be replaced by DIR mydirectoryvariable or should it be DIR VALUE.

I'm not sure, I will check this.

Are there any limitations on the value of mydirectoryvariable can be, like can it be SESSION:TEMP-DIR or does it have to be a (sub)directory of the upload directory of the Jetty server and if so how can we get that path from 4GL in FWD.

The directory can be any character expression. The specified directory must exist and the FWD client Java process must have system rights to write to the directory. When relative path is specified it will be relative to the working directory of the FWD client Java process.

What does "MULTIPLE option currently only works for the web uploader" mean,

You can select/upload multiple files at once.

what is a web uploader?

By web uploader I mean the web UI dialog that is used select files.

Wow thanks that's a very quick response :-)

Hynek Cihlar wrote:

The directory can be any character expression. The specified directory must exist and the FWD client Java process must have system rights to write to the directory. When relative path is specified it will be relative to the working directory of the FWD client Java process.

So from 4GL, how do I determine a valid value? Is the working directory of the FWD client Java process what you get from FILE-INFO:FULL-PATHNAME after FILE-INFO:FILE-NAME = "." ? Given security concerns, would you recommend that files are uploaded to the working directory of the FWD client Java process?

Hynek Cihlar wrote:

The specified directory must exist and the FWD client Java process must have system rights to write to the directory.

Isn't that always true for SESSION:TEMP-DIR?

So rather than uploading to the working directory of the FWD client Java process, is it always true that SYSTEM-DIALOG can upload to (a subdirectory of) the SESSION:TEMP-DIR ?

An other, related question: what happens when the mydirectoryvariable (in UPLOAD DIR mydirectoryvariable) happens to have value "" or the unknown value?
Will that throw a catcheable error? Or will variable okpressed in UPDATE okpressed become false? Or will mycharactervariable become "" or the unknown value? Or some other effect?

Jurjen Dijkstra wrote:

Hynek Cihlar wrote:

The specified directory must exist and the FWD client Java process must have system rights to write to the directory.

Isn't that always true for SESSION:TEMP-DIR?

I suppose it should. But with IO you never know. There could be a temporary storage error, full disk, etc.

So rather than uploading to the working directory of the FWD client Java process, is it always true that SYSTEM-DIALOG can upload to (a subdirectory of) the SESSION:TEMP-DIR ?

AFAIK all the file IO in FWD (and I suppose OE, too) resolving on the client are relative to the working dir or propath entries. If you want to make the path relative to SESSION:TEMP-DIR, you can use this value when setting DIR.

An other, related question: what happens when the mydirectoryvariable (in UPLOAD DIR mydirectoryvariable) happens to have value "" or the unknown value?
Will that throw a catcheable error? Or will variable okpressed in UPDATE okpressed become false? Or will mycharactervariable become "" or the unknown value? Or some other effect?

If the value is unknown or the resolved dir is not an existing directory, the UPDATE variable is set to false. In this case the other "output" variables are not touched.

Hynek Cihlar wrote:

So rather than uploading to the working directory of the FWD client Java process, is it always true that SYSTEM-DIALOG can upload to (a subdirectory of) the SESSION:TEMP-DIR ?

AFAIK all the file IO in FWD (and I suppose OE, too) resolving on the client are relative to the working dir or propath entries. If you want to make the path relative to SESSION:TEMP-DIR, you can use this value when setting DIR.

Sorry I don't understand the answer.
Allow me to rephrase the question.

Suppose for example that SESSION:TEMP-DIR is "/v1/tmp". As you can see this is NOT a relative path, but an absolute path. It is also not one of the propath entries and it is also not the working dir. But it is SESSION:TEMP-DIR so the OpenEdge session is writing temporary files there, so evidently the process has permissions to do file read/writes. I mean, the OpenEdge process has. So I am assuming that the FWD process also those permissions and if the actual file upload is handled by the embedded Jetty server then I assume that this Jetty also has permission to write in "/v1/tmp". But that's just my assumption, and I want to verify my assumption. Hence the question.

So if I would (using 4GL statements) create a subdirectory SESSION:TEMP-DIR + "/fwduploads"
then assign variable fwduploaddir = SESSION:TEMP-DIR + "/fwduploads"
then call statement SYSTEM-DIALOG GET-FILE AT-WEB-BROWSER UPLOAD DIR fwduploaddir

... can you please confirm with a yes or no answer that it works?

If the value is unknown or the resolved dir is not an existing directory, the UPDATE variable is set to false. In this case the other "output" variables are not touched.

That's cool. Thanks.

Jurjen Dijkstra wrote:

So if I would (using 4GL statements) create a subdirectory SESSION:TEMP-DIR + "/fwduploads"
then assign variable fwduploaddir = SESSION:TEMP-DIR + "/fwduploads"
then call statement SYSTEM-DIALOG GET-FILE AT-WEB-BROWSER UPLOAD DIR fwduploaddir

... can you please confirm with a yes or no answer that it works?

Yes, this will work fine.

Thanks!!
Now I can confidently rewrite the 4GL code to use the new feature.

I have used SYSTEM-DIALOG with the new enhancements in 4GL source and am testing the outcome. It appears that mycharactervariable and myremotecharactervariable (from comment #12) are behaving exactly opposite to what I expected.

My 4GL sourcecode is:

        fwduploaddir = "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba". // actually there is a method that returns this value

        system-dialog get-file filenaam 
           at-web-browser clientfilenaam 
           upload dir fwduploaddir
           title   "Kies het rapport archief":t
           filters "zip archief (*.zip)":t   "*.zip":u
           must-exist 
           update lOk.

This converts to:

               FileDialog.createFileSearchDialog(filenaam)
                     .atWebBrowser(clientfilenaam)
                     .upload(fwdUploadDir)
                     .setTitle("Kies het rapport archief")
                     .setFilters("zip archief (*.zip)", "*.zip")
                     .mustExist()
                     .update(lOk)
                     .execute();

When I run it, and select file "c:\temp\rapzip.zip" on my PC and then click the Ok button, then:

• filenaam has value "rapzip.zip" but I expected "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip"
• clientfilenaam has value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" but I expected "c:\temp\rapzip.zip"

Is my expectation wrong or is the explanation in comment 12 wrong?

By the way file "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" actually exists on the server so the upload has succeded, that's cool!
Only the two output parameters seems to be swapped. Oh and one is returning the filename "rapzip.zip" while it should have been fully qualified as "c:\temp\rapzip.zip"

Jurjen Dijkstra wrote:

By the way file "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" actually exists on the server so the upload has succeded, that's cool!
Only the two output parameters seems to be swapped. Oh and one is returning the filename "rapzip.zip" while it should have been fully qualified as "c:\temp\rapzip.zip"

Unfortunately there is no way to get the absolute file path on the client. This is a security measure implemented by web browsers. Given this limitation I don't think there is a point having this remote output variable at all.

Hynek Cihlar wrote:

Unfortunately there is no way to get the absolute file path on the client. This is a security measure implemented by web browsers. Given this limitation I don't think there is a point having this remote output variable at all.

O dear. Well if there is no way, and if the temp-file on the server has the same filename as on the PC (in my test "rapzip.zip") then I have to agree that this remote output variable is pointless. I can just didsplay sys.filesystem:Filename(filenaam) in the dialog as a visual feedback to the user that he indeed selected a file.

So now how to continue?
It seems to me that parameter filenaam in FileDialog.createFileSearchDialog(filenaam) should get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" and that parameter clientfilenaam will be removed?

Jurjen Dijkstra wrote:

So now how to continue?
It seems to me that parameter filenaam in FileDialog.createFileSearchDialog(filenaam) should get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" and that parameter clientfilenaam will be removed?

Correct.

I removed the remote file lvalue option from AT-WEB-BROWSER in 3809e revision 11428.

The expected syntax is now: SYSTEM-DIALOG GET-FILE mycharactervariable MULTIPLE AT-WEB-BROWSER UPLOAD DIR "/tmp". Where MULTIPLE, UPLOAD and DIR are optional.

Code Review Task Branch 3809e Revision 11428

The changes are good.

Hynek: Please write up the documentation in SYSTEM-DIALOG GET-FILE AT-WEB-BROWSER UPLOAD

#4437-10

Hynek, it seems that instanceof can be used for checking this condition data.constructor !== "Uint8Array" because the right side operand is a function object and it always returns true. (This change is in the function send() defined in p2j.socket.js)

Sergey Ivanovskiy wrote:

Hynek, it seems that instanceof can be used for checking this condition data.constructor !== "Uint8Array" because the right side operand is a function object and it always returns true. (This change is in the function send() defined in p2j.socket.js)

True. The expression should have been data.constructor.name !== "Uint8Array" or data instanceof Uint8Array as you point out. I will change it to use instanceof. However this should not be the cause of this issue, correct?

Yes, correct and the upload dialog shouldn't cause it too.

I found that this change is not clear for me

       StringBuilder sb = new StringBuilder();

-      for (int i = offset; i < length; i += 2)
+      for (int i = 0; i < 2*length; i += 2)
       {
-         sb.append((char) readMessageInt16(message, i));
+         sb.append((char) readMessageInt16(message, i + offset));
       }

It is used by

   public void paste(byte[] message, int offset, int length)
   {
      if (isVT100)
      {
         super.paste(message, offset, length);
         return;
      }

      // save off the clipboard contents
      synchronized (clipboardLock)
      {
         pending = ClipboardRequest.CLIENT_GENERATED;

         // handle the special case of an empty clipboard text
         if (length == 3 && message[offset + 1] == 0 && message[offset + 2] == 22)
         {
            clipboardContents = "";
         }
         else
         {
            clipboardContents = readMessageText(message, offset + 1, length);
         }

Hynek, I checked that this change causes copy, paste issue.

I think that this function readMessageText worked correctly before you changed it. This diff rolled it back and fixed the copy and paste issue. Also it should work properly with MSG_FILE_UPLOADING code

=== modified file 'src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java'
--- src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java    2019-11-20 17:14:45 +0000
+++ src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java    2019-12-13 14:03:37 +0000
@@ -692,9 +692,9 @@
    {
       StringBuilder sb = new StringBuilder();

-      for (int i = 0; i < 2*length; i += 2)
+      for (int i = offset; i < length; i += 2)
       {
-         sb.append((char) readMessageInt16(message, i + offset));
+         sb.append((char) readMessageInt16(message, i));
       }

       return sb.toString();

Sergey Ivanovskiy wrote:

I think that this function readMessageText worked correctly before you changed it. This diff rolled it back and fixed the copy and paste issue. Also it should work properly with MSG_FILE_UPLOADING code

[...]

I'm glad you found it!

I interpreted the length param as string length and not as byte length as I should have. Can you please extend the javadoc of the param to make this more clear, so that we can prevent this kind of error in the future? Also if you are going to commit the change, also change the two references of readMessageText when processing MSG_FILE_CHOOSE message, so that the length argument is multiplied by 2.

It seems all usages are correct now. The java doc states that length is "Location in the message where the text ends." It describes this case correctly. readMessageText has these usages:

GuiWebSocket
paste(byte[], int, int)
processBinaryMessage(byte[], int, int) (23 matches)
processChannel(FileChannel)

  /**
    * Interpret bytes from a binary message as text and return the accumulated result.
    *
    * @param    message
    *           Message content as an byte array.
    * @param    offset
    *           Offset into the message at which the text begins.
    * @param    length
    *           Location in the message where the text ends.
    *
    * @return   The message text.
    */
   public String readMessageText(byte[] message, int offset, int length)

Instead of length we can substitute end. So the final diff can be this one

=== modified file 'src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java'
--- src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java    2019-11-20 17:14:45 +0000
+++ src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java    2019-12-13 20:05:27 +0000
@@ -683,18 +683,18 @@
     *           Message content as an byte array.
     * @param    offset
     *           Offset into the message at which the text begins.
-    * @param    length
+    * @param    end
     *           Location in the message where the text ends.
     *
     * @return   The message text.
     */
-   public String readMessageText(byte[] message, int offset, int length)
+   public String readMessageText(byte[] message, int offset, int end)
    {
       StringBuilder sb = new StringBuilder();

-      for (int i = 0; i < 2*length; i += 2)
+      for (int i = offset; i < end; i += 2)
       {
-         sb.append((char) readMessageInt16(message, i + offset));
+         sb.append((char) readMessageInt16(message, i));
       }

       return sb.toString();

Sergey Ivanovskiy wrote:

It seems all usages are correct now. The java doc states that length is "Location in the message where the text ends." It describes this case correctly. readMessageText has these usages:

I think the location can be interpreted as character location. This happened to me anyway. I will update the javadoc.

[...]
All of them use this function correctly.
[...]

There are two occurrences where the function is not used correctly:

selectedFiles[i] = readMessageText(message, offset, len);

where len is character length.

I'll fix these.

OK, thank you.

This is fixed in 3809e revision 11444. Sergey, please review the change.

The changes in rev 11444 (3809e) are good. The found issue #4437 was fixed.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek, it seems that instanceof can be used for checking this condition data.constructor !== "Uint8Array" because the right side operand is a function object and it always returns true. (This change is in the function send() defined in p2j.socket.js)

True. The expression should have been data.constructor.name !== "Uint8Array" or data instanceof Uint8Array as you point out. I will change it to use instanceof. However this should not be the cause of this issue, correct?

It seems that we must fix this code, otherwise for each message new Uint8Array will be created.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek, it seems that instanceof can be used for checking this condition data.constructor !== "Uint8Array" because the right side operand is a function object and it always returns true. (This change is in the function send() defined in p2j.socket.js)

True. The expression should have been data.constructor.name !== "Uint8Array" or data instanceof Uint8Array as you point out. I will change it to use instanceof. However this should not be the cause of this issue, correct?

It seems that we must fix this code, otherwise for each message new Uint8Array will be created.

This is resolved in 3809e revision 11446.

Hynek Cihlar wrote:

Jurjen Dijkstra wrote:

So now how to continue?
It seems to me that parameter filenaam in FileDialog.createFileSearchDialog(filenaam) should get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" and that parameter clientfilenaam will be removed?

Correct.

We have received the new FWD version last weekend, now I am using it. It does not seem to work as expected.

when upload dir is ""/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba" and when I select file "c:\temp\rapzip.zip", then I expect to get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip".

But what happened is that the returned value is "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/5b461ab6cd45e48f01e2744". It does not make sense to me. It is not even inside the specified upload dir. I don't know what "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/5b461ab6cd45e48f01e2744" is, maybe it is a directory, maybe a file with no extension, maybe it does not exist on the filesystem.

Jurjen Dijkstra wrote:

Hynek Cihlar wrote:

Jurjen Dijkstra wrote:

So now how to continue?
It seems to me that parameter filenaam in FileDialog.createFileSearchDialog(filenaam) should get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" and that parameter clientfilenaam will be removed?

Correct.

We have received the new FWD version last weekend, now I am using it. It does not seem to work as expected.

when upload dir is ""/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba" and when I select file "c:\temp\rapzip.zip", then I expect to get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip".

But what happened is that the returned value is "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/5b461ab6cd45e48f01e2744". It does not make sense to me. It is not even inside the specified upload dir. I don't know what "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/5b461ab6cd45e48f01e2744" is, maybe it is a directory, maybe a file with no extension, maybe it does not exist on the filesystem.

When the upload dir is specified, there is not much logic that would alter the actual dir down the path. Is the file actually uploaded? Do you see any errors in the client (or server) logs? Also note that if the upload fails, the path variable is not assigned, so you may read a stale value.

Also the "filters" argument does not have any effect. I am specifying
filters "zip archive (*.zip)" "*.zip"
but that does nothing, just as if I did not specify filters.

Also, when statement system-dialog get-file executes, it first displays a useless dialog with nothing but two buttons: "Select file " and "Cancel". Why would anyone cancel at this point. Can you hide this useless dialog please?

Jurjen Dijkstra wrote:

Also the "filters" argument does not have any effect. I am specifying
filters "zip archive (*.zip)" "*.zip"
but that does nothing, just as if I did not specify filters.

Also, when statement system-dialog get-file executes, it first displays a useless dialog with nothing but two buttons: "Select file " and "Cancel". Why would anyone cancel at this point. Can you hide this useless dialog please?

This dialog is a stripped down version of the upload JS dialog when no multiple option is given to the statement. The dialog is needed for the security reasons - the local file access must be originated from a user action.

The cancel button is needed in case you choose to cancel the upload operation.

Jurjen Dijkstra wrote:

Also the "filters" argument does not have any effect. I am specifying
filters "zip archive (*.zip)" "*.zip"

Filters are not supported in the current implementation. But this will be trivial to add.

Hynek Cihlar wrote:

Jurjen Dijkstra wrote:

Also, when statement system-dialog get-file executes, it first displays a useless dialog with nothing but two buttons: "Select file " and "Cancel". Why would anyone cancel at this point. Can you hide this useless dialog please?

This dialog is a stripped down version of the upload JS dialog when no multiple option is given to the statement.

Yes I know. It is stripped down to only two buttons, "select file" and "cancel" both of which make no sense at this point in the workflow. After all, user has just pressed a "select file" button in the converted 4GL UI, so from a UI standpoint it is now equivalent to the hated "are you sure" dialog.

The dialog is needed for the security reasons - the local file access must be originated from a user action.

It is already originated from a user action. The user just did something in the converted 4GL UI to execute the system-dialog statement.

The cancel button is needed in case you choose to cancel the upload operation.

I disagree, it is not needed here :-) Ok maybe with the multiple option if the user wants to abort a long upload, but even in that case the cancel button only makes sense after the user returns from the filepicker and the upload operation is in progress. It does not make sense to cancel before the filepicker even started especially since there is also a cancel button in the filepicker itself.

Jurjen Dijkstra wrote:

Hynek Cihlar wrote:
The dialog is needed for the security reasons - the local file access must be originated from a user action.

It is already originated from a user action. The user just did something in the converted 4GL UI to execute the system-dialog statement.

Yes, but the action was performed in the context of the converted application logic, not in the context of the Javascript engine in the browser. In other words the browser has no way to interpret the 4GL button press as a file-related action.

I disagree, it is not needed here :-) Ok maybe with the multiple option if the user wants to abort a long upload, but even in that case the cancel button only makes sense after the user returns from the filepicker and the upload operation is in progress. It does not make sense to cancel before the filepicker even started especially since there is also a cancel button in the filepicker itself.

I agree the Cancel button adds a bit of complexity to the UI. On the other hand, if the Cancel button is removed, the user may be confused in cases he chooses to cancel the operation - 1. he may be unaware of the upcoming browser upload dialog, 2. it is an extra step and click to cancel the operation.

Hynek Cihlar wrote:

Jurjen Dijkstra wrote:

Hynek Cihlar wrote:

Jurjen Dijkstra wrote:

So now how to continue?
It seems to me that parameter filenaam in FileDialog.createFileSearchDialog(filenaam) should get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" and that parameter clientfilenaam will be removed?

Correct.

We have received the new FWD version last weekend, now I am using it. It does not seem to work as expected.

when upload dir is ""/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba" and when I select file "c:\temp\rapzip.zip", then I expect to get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip".

But what happened is that the returned value is "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/5b461ab6cd45e48f01e2744". It does not make sense to me. It is not even inside the specified upload dir. I don't know what "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/5b461ab6cd45e48f01e2744" is, maybe it is a directory, maybe a file with no extension, maybe it does not exist on the filesystem.

When the upload dir is specified, there is not much logic that would alter the actual dir down the path. Is the file actually uploaded? Do you see any errors in the client (or server) logs? Also note that if the upload fails, the path variable is not assigned, so you may read a stale value.

Well the upload dir is specified and this is what happened, so evidently there is still some logic that alters it.

I don't know if the file was actually uploaded. The variable was assigned and the logical returnvalue was true. The system-dialog get-file statement did not have the no-error option and no error was thrown. So in 4GL there was no reason to believe it failed or aborted.

Jurjen Dijkstra wrote:

Hynek Cihlar wrote:

Jurjen Dijkstra wrote:

Hynek Cihlar wrote:

Jurjen Dijkstra wrote:

So now how to continue?
It seems to me that parameter filenaam in FileDialog.createFileSearchDialog(filenaam) should get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip" and that parameter clientfilenaam will be removed?

Correct.

We have received the new FWD version last weekend, now I am using it. It does not seem to work as expected.

when upload dir is ""/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba" and when I select file "c:\temp\rapzip.zip", then I expect to get value "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/014b7619b665035e4a6f1219319c29ba/rapzip.zip".

But what happened is that the returned value is "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/5b461ab6cd45e48f01e2744". It does not make sense to me. It is not even inside the specified upload dir. I don't know what "/tmp/mtctmp/stsjafhvjkhbdpgkjk/fwdupload/5b461ab6cd45e48f01e2744" is, maybe it is a directory, maybe a file with no extension, maybe it does not exist on the filesystem.

When the upload dir is specified, there is not much logic that would alter the actual dir down the path. Is the file actually uploaded? Do you see any errors in the client (or server) logs? Also note that if the upload fails, the path variable is not assigned, so you may read a stale value.

Well the upload dir is specified and this is what happened, so evidently there is still some logic that alters it.

If you have the issue isolated in a compact piece of code, please post it here. If not post at least the system-dialog statement.

I don't know if the file was actually uploaded. The variable was assigned and the logical returnvalue was true. The system-dialog get-file statement did not have the no-error option and no error was thrown. So in 4GL there was no reason to believe it failed or aborted.

Can you check on the file system if the file is there?

Sure, I will see if I can collect more data. This afternoon I will not have time for that unfortunately, so it'll be Monday.

I see what the problem is: the string is cut at 64 characters.

The upload directory that I have specified is: "/tmp/mtctmp/stddbnadcdrpkbflol/fwdupload/d0490d8ecdc7bc7a708b8fc4b80d2ab1"
In the system-dialog I have selected file "c:\temp\status.txt"

On Linux I can see that file "/tmp/mtctmp/stddbnadcdrpkbflol/fwdupload/d0490d8ecdc7bc7a708b8fc4b80d2ab1/status.txt" has been uploaded and the filecontents is correct.

But the filename parameter, that is returned from system-dialog get-file has value "/tmp/mtctmp/stddbnadcdrpkbflol/fwdupload/d0490d8ecdc7bc7a708b8fc".

That's only the first 64 characters of "/tmp/mtctmp/stddbnadcdrpkbflol/fwdupload/d0490d8ecdc7bc7a708b8fc4b80d2ab1/status.txt".

Jurjen Dijkstra wrote:

I see what the problem is: the string is cut at 64 characters.

The upload directory that I have specified is: "/tmp/mtctmp/stddbnadcdrpkbflol/fwdupload/d0490d8ecdc7bc7a708b8fc4b80d2ab1"
In the system-dialog I have selected file "c:\temp\status.txt"

On Linux I can see that file "/tmp/mtctmp/stddbnadcdrpkbflol/fwdupload/d0490d8ecdc7bc7a708b8fc4b80d2ab1/status.txt" has been uploaded and the filecontents is correct.

But the filename parameter, that is returned from system-dialog get-file has value "/tmp/mtctmp/stddbnadcdrpkbflol/fwdupload/d0490d8ecdc7bc7a708b8fc".

That's only the first 64 characters of "/tmp/mtctmp/stddbnadcdrpkbflol/fwdupload/d0490d8ecdc7bc7a708b8fc4b80d2ab1/status.txt".

any reaction?

Jurjen Dijkstra wrote:

any reaction?

Sorry, I was busy with other tasks. I will check the sources where the value gets trimmed and then let you know.

ok I was just worried that the mail was lost

Yes, go ahead.

Hynek Cihlar wrote:

Hynek Cihlar wrote:

Jurjen Dijkstra wrote:

any reaction?

Sorry, I was busy with other tasks. I will check the sources where the value gets trimmed and then let you know.

I found the cause and fixed it. The change is attached.

Greg, this is safe to be included in 3809e if you agree.

Is it safe? What are usages? The java doc is that

    * Interpret bytes from a binary message as text and return the accumulated result.
    *
    * @param    message
    *           Message content as an byte array.
    * @param    offset
    *           Offset into the message at which the text begins.
    * @param    length
    *           Location in the message where the text ends. Note that this is a byte location, pass in the
    *           character length of the text to read multiplied by 2.
    *
    * @return   The message text.

It is incorrect change.

Sergey Ivanovskiy wrote:

It is incorrect change.

Can you provide more details?

Hynek Cihlar wrote:

I found the cause and fixed it. The change is attached.

Checked in to 3809e revision 11471.

Hynek Cihlar wrote:

Hynek Cihlar wrote:

I found the cause and fixed it. The change is attached.

Checked in to 3809e revision 11471.

Please remove this fix

=== modified file 'src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java'
--- src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java    2019-12-13 20:34:46 +0000
+++ src/com/goldencode/p2j/ui/client/driver/web/WebClientProtocol.java    2020-01-16 22:00:48 +0000
@@ -693,7 +693,7 @@
    {
       StringBuilder sb = new StringBuilder();

-      for (int i = offset; i < length; i += 2)
+      for (int i = offset; i < length + offset; i += 2)
       {
          sb.append((char) readMessageInt16(message, i));
       }

   /**
    * Interpret bytes from a binary message as text and return the accumulated result.
    *
    * @param    message
    *           Message content as an byte array.
    * @param    offset
    *           Offset into the message at which the text begins.
    * @param    length
    *           Location in the message where the text ends. Note that this is a byte location, pass in the
    *           character length of the text to read multiplied by 2.
    *
    * @return   The message text.
    */
   public String readMessageText(byte[] message, int offset, int length)

Sergey Ivanovskiy wrote:

this change is incorrect. Please look at all its usages and fix the new incorrect usages of this function
[...]
We discussed this before when the similar changes in this function brake copy and paste functionality.
The length defines the end of the byte block within the message but not the length of the target text. We should rename this parameter length to be endPosition.

I see and am glad you spot it. Please review 3809e revision 11472. I also renamed the parameter names and updated the method's javadoc.

3809e revision 11472 has good changes.

It seems that this parameter name length came from the web socket api. This web socket connection listener can confuse

void onWebSocketBinary​(byte[] payload, int offset, int len)

A WebSocket binary frame has been received.

Parameters:
    payload - the raw payload array received
    offset - the offset in the payload array where the data starts
    len - the length of bytes in the payload 

3809e was merged to trunk as revision 11340. If I understand correctly, the truncation of the filename is now fixed in trunk. Is that right?

Hynek: Please write up the documentation in SYSTEM-DIALOG GET-FILE AT-WEB-BROWSER UPLOAD.

Greg Shah wrote:

3809e was merged to trunk as revision 11340. If I understand correctly, the truncation of the filename is now fixed in trunk. Is that right?

Yes, this is now resolved in trunk.

Hynek: Please write up the documentation in SYSTEM-DIALOG GET-FILE AT-WEB-BROWSER UPLOAD.

Will do.

Is the AT-WEB-BROWSER <filename> in trunk? I am getting unexpected token: <filename> on my conversion.

Roger Borrello wrote:

Is the AT-WEB-BROWSER <filename> in trunk? I am getting unexpected token: <filename> on my conversion.

The lvalue option was removed from AT-WEB-BROWSER, see #4113-33. You can see the full syntax at SYSTEM-DIALOG_GET-FILE_AT-WEB-BROWSER_UPLOAD.

Hynek Cihlar wrote:

Greg Shah wrote:

Hynek: Please write up the documentation in SYSTEM-DIALOG GET-FILE AT-WEB-BROWSER UPLOAD.

Will do.

Please review SYSTEM-DIALOG_GET-FILE_AT-WEB-BROWSER_UPLOAD.

Please review SYSTEM-DIALOG_GET-FILE_AT-WEB-BROWSER_UPLOAD.

In case MULTIPLE is used I would like to know what the format is of mycharactervariable. Can you document that too please?
I have not had time to actually test this myself, so I can only guess and debug. My first guess would be that mycharactervariable is a comma-separated string of all absolute filenames. My second guess is that the format is similar to the lpstrFile member of the OPENFILENAMEA structure in ms windows, where the string is not comma separated but null separated and the first entry is the directory while the other entries are relative filenames.

Also I think it is worth to mention that option FILTERS does not work (yet).
And that mycharactervariable is OUTPUT and not INPUT-OUTPUT and that INITIAL-DIR does not work in the browser.

I think it is also important to note that mycharactervariable contains the absolute filename of the uploaded file (in the upload tempdir) and there is no way to get the name of the absolute filename of the client.

Please review SYSTEM-DIALOG_GET-FILE_AT-WEB-BROWSER_UPLOAD.

In case MULTIPLE is used I would like to know what the format is of mycharactervariable. Can you document that too please?
I have not had time to actually test this myself, so I can only guess and debug. My first guess would be that mycharactervariable is a comma-separated string of all absolute filenames. My second guess is that the format is similar to the lpstrFile member of the OPENFILENAMEA structure in ms windows, where the string is not comma separated but null separated and the first entry is the directory while the other entries are relative filenames.

I think it is also important to note that mycharactervariable contains the absolute filename of the uploaded file (in the upload tempdir) and there is no way to get the name of the absolute filename of the client.

The text suggests that one can use the AT-WEB-BROWSER option without using the UPLOAD option. I wonder, when you would do that and select a file, will mycharactervariable in that case contain the absolute filename of the file on the client?

Would it not be easier to read (or less ambiguous) when the same notation was used as in OpenEdge help, e.g.

SYSTEM-DIALOG GET-FILE mycharactervariable [MULTIPLE] [AT-WEB-BROWSER] [UPLOAD [DIR "/tmp"]]

or is it:

SYSTEM-DIALOG GET-FILE mycharactervariable [MULTIPLE] [AT-WEB-BROWSER [UPLOAD [DIR "/tmp"]]]

And what happened to all the other options that are available in OpenEdge ABL:

SYSTEM-DIALOG GET-FILE character-field
[ FILTERS name filespec
[ , name filespec ] ...
[ INITIAL-FILTER filter-num ]
]
[ ASK-OVERWRITE ]
[ CREATE-TEST-FILE ]
[ DEFAULT-EXTENSION extension-string ]
[ INITIAL-DIR directory-string ]
[ MUST-EXIST ]
[ RETURN-TO-START-DIR ]
[ SAVE-AS ]
[ TITLE title-string]
[ USE-FILENAME ]
[ UPDATE logical-variable ]
[ IN WINDOW window ]


Do all these options still work? Do they work differently when the AT-WEB-BROWSER option is used? I found that FILTERS does not work (yet) and INITIAL-DIR fails by design and I believe that these little facts should be documented.

Jurjen Dijkstra wrote:

Please review SYSTEM-DIALOG_GET-FILE_AT-WEB-BROWSER_UPLOAD.

In case MULTIPLE is used I would like to know what the format is of mycharactervariable. Can you document that too please?

Jurjen, thanks for your valuable suggestions. I updated the documentation, please see SYSTEM-DIALOG_GET-FILE_AT-WEB-BROWSER_UPLOAD.

Thanks!

I am confused... without UPLOAD, the mycharactervariable will be assigned to the file name of the selected file. But with UPLOAD there is no way to get the file name due to the security limitations of the web browser?

I understand there may be security limitations but I don't understand that these security limitations only apply when the UPLOAD option is specified.

Jurjen Dijkstra wrote:

Thanks!

I am confused... without UPLOAD, the mycharactervariable will be assigned to the file name of the selected file. But with UPLOAD there is no way to get the file name due to the security limitations of the web browser?

I understand there may be security limitations but I don't understand that these security limitations only apply when the UPLOAD option is specified.

There is no way to get to the full file path on the client system (where the browser runs) regardless of the UPLOAD option. This statement is not mentioned together with the UPLOAD option in one sentence so I think this should be clear. Regardless I changed the wording of the paragraph a bit, hopefully it is more clear now.

:-) it is more clear now.

Hynek: I've reviewed the document and made some additional edits. I restructured some things and also added the FILTERS and INITIAL-FILTER to the list of unsupported options. Please review to confirm it is OK.

Greg Shah wrote:

Hynek: I've reviewed the document and made some additional edits. I restructured some things and also added the FILTERS and INITIAL-FILTER to the list of unsupported options. Please review to confirm it is OK.

It is a lot better.

Roger Borrello wrote:

Right now they are looking to UPDATE logical-value for an indication of whether or not a file transfer took place, which I don't believe is correct. Is there any indication of success or failure related to a file copy?

This is how AT-WEB-BROWSER behaves, when the operation succeeds the value is set to TRUE, otherwise it is set to FALSE. So if the file is uploaded OK the UPDATE lvalue will be set to TRUE. Do you see something else?

Also, is there any need to perform a delete of the file returned for "cleanup"?

Yes, the cleanup must be handled by the 4GL application logic.

Hynek Cihlar wrote:

Roger Borrello wrote:

Right now they are looking to UPDATE logical-value for an indication of whether or not a file transfer took place, which I don't believe is correct. Is there any indication of success or failure related to a file copy?

This is how AT-WEB-BROWSER behaves, when the operation succeeds the value is set to TRUE, otherwise it is set to FALSE. So if the file is uploaded OK the UPDATE lvalue will be set to TRUE. Do you see something else?

I have not done any runtime, so I will utilize your information. Right now we are just performing conversion.

A follow-up...

GET-FILE filename
.
.
.
USE-FILENAME
AT-WEB-BROWSER
UPDATE success

Roger Borrello wrote:

The value of filename is the same as what was passed in upon return, regardless of success or failure, correct?

It will be the same only on failure, i.e. when the UPDATE lvalue is FALSE. On success, it will be set to the selected file(s).