FWD » Core Development » Base Language

• ensure deploy/server/directory.xml and deploy/server/server.xml are properly configured with the right paths, DB connections and OS names if needed (i.e. sv). Check for any rfb occurrence - that stands for Roger and marks a hardcoded value.
• ensure the server opens after ./server.sh. see the ports using ./server.sh -t (usually s: 3333 and i: 3433)
• ensure the server starts and prints Server started in the logs. There should be no exception or fatal crash.
• configure deploy/client/unit-test.xml with the proper server ports (s: 3333 and i: 3433)
• check junit.sh for any obvious problems inside it; check if there are any hardcoded paths (usually starting containing rfb for Roger). If so, consider rewritting the paths to match your environment
• check if there is anything popping up in the server log when attempting to run junit. If there is no new logs, it means that client can't connect to the server (that is why I am stressing the importance of port setting).
  • also ensure that unit-test.xml has localhost as server host.

Managed to solve it after following your steps, thanks.

Managed to fix this testcase from yesterday, that checks for I18N encoding, a sample:

    @Test.
    method public void LoadMemptrSameEncoding ():
        define variable hNode as HANDLE no-undo.
        define variable memptrXml as memptr no-undo.
        CREATE X-NODEREF hNode.

        set-size(memptrXml) = 150.
        put-string(memptrXml, 1) = '<?xml version="1.0" encoding="ISO-8859-1" standalone="no"?><references>"àáâãäå"</references>'.
        xDocument:load("memptr", memptrXml, no).

        Assert:Equals ("ISO-8859-1", xDocument:ENCODING).
        Assert:IsTrue (xDocument:GET-CHILD (hNode, 1)).
        Assert:IsTrue (hNode:GET-CHILD (hNode, 1)).
        Assert:Equals ('"àáâãäå"', hNode:NODE-VALUE).
    end method.

loadNoArgsNoSuppress ✘ Expected: **The LOAD attribute on the X-DOCUMENT widget has invalid arguments. (4065) but was: ** The LOAD attribute on the X-DOCUMENT widget has invalid arguments. (4065)

Another issue that I didn't have the chance to look into was why the catch block is not reached here:

    @Test.
    method public void LoadNoArgsSuppress ( ):
        define variable errMsg as character no-undo.
        define variable lok as logical no-undo initial true.
        errMsg = 'Argument for LOAD/SAVE must be ~'file~', ~'stream~', ~'stream-handle~' or ~'memptr~'. (9170)'.
        session:suppress-warnings = true no-error.
        xDocument:load("", "", no).
        Assert:isFalse(lok).

        CATCH ex AS Progress.Lang.Error:
            Assert:Equals(1, ex:NumMessages).
            Assert:Equals(9170, ex:GetMessageNum(1)).
            Assert:Equals(errMsg, ex:GetMessage(1)).
        END CATCH.

        FINALLY:
                session:suppress-warnings = false no-error.
        END FINALLY.
    end method.

loadNoArgsSuppress ✘ Expected: 9170 but was: 0

After investigating it turns out that the load() function that is called in the converted code adds the prefix " ", thus resulting in ** + " " + "The LOAD attribute on the X-DOCUMENT widget has invalid arguments. (4065)"

FWD should be compatible with 4GL in terms of error messages, so this is a bug too. There are cases when the error is actually caught, the message parsed and a decision is taken based on that. If FWD doesn't mimic that message exactly, the execution path may diverge. So this should be fixed.

If the assert is left there, it's entering the catch, and it gets this assertion's error message, resulting in:

So you are stating that the catch is actually catching the AssertError instead of the desired failed loading error. This is a bug; it means that FWD is not failing on xDocument:load, but rather continues execution and it crashes due to a failed assertion. The goal is to make :load also throw the error due to the fact that the arguments are invalid. TL;DR: FWD succeeds in loading, but 4GL fails. We need to make FWD fail as well.

After looking with Danut yesterday we have found that the prefix is added here:

   public static String buildErrorText(int num, String text, boolean prefix, boolean addDot)
   {
      // build up the message with StringBuilder rather than String.format, as this code may
      // be called quite often in normal control flow, and String.format is expensive
      StringBuilder buf = (prefix ? new StringBuilder("** ") : new StringBuilder());
      buf.append(text);

      // SysError with error number 0 does not add this
      if (num > 0)
      {
         buf.append(addDot ? ". (" : " (");
         buf.append(num);
         buf.append(')');
      }

      return buf.toString();

Alexandru Lungu wrote:

If the assert is left there, it's entering the catch, and it gets this assertion's error message, resulting in:

So you are stating that the catch is actually catching the AssertError instead of the desired failed loading error. This is a bug; it means that FWD is not failing on xDocument:load, but rather continues execution and it crashes due to a failed assertion. The goal is to make :load also throw the error due to the fact that the arguments are invalid. TL;DR: FWD succeeds in loading, but 4GL fails. We need to make FWD fail as well.

I'm not sure about this, but:

   private void invalidReadWriteModeError(String attribute)
   {
      String msg = "Argument for LOAD/SAVE must be 'file', 'stream', " +
              "'stream-handle' or 'memptr'";
      ErrorManager.recordOrShowError(9170, msg, false, false, false); // here

      invalidArgumentError(attribute);
   }

This is taken directly from XDocumentImpl and I think the last argument, which is isError, should be true because in 4gl this error is caught inside the catch, but here it's behaving like a warning (from what I'm understanding).

ErrorManager is the right place to look at when it comes down to error handling. It has several endpoints (record / throw / show or a mix of these) with several arguments (with suffix, with error message, with error code, etc.) Please test and find the best endpoind/parameter combination for your issue to match 4GL. For that, you can use 4GL examples to show-cast its behavior: CATCH blocks, observation of alert boxes and their messages, how it behaves in regard to NO-ERROR and the control-flow (if it continues to execute next statement). This investigation should build up knowledge over the error handling in 4GL and how FWD replicates it.

PS: I think recordOrThrowError is more suitable. recordOrShowError will simply display the alert-box, but it won't throw any exception and will continue execution. The "record" term is used for cases when the statement is suffixed with NO-ERROR. In that case, 4GL will simply record the error in the error list, accessible in ERROR-STATUS system handle. You can check that. If the ERROR-STATUS is not updated in NO-ERROR mode, I suspect ErrorManager.throwError() is even more suitable (very improbable).

When working on another tc, I saw that the error message thrown by 4gl is different from the FWD one.

    @Test.
    method public void LoadMemptrWrongEncoding ():
        define variable lok as logical no-undo initial true.
        define variable errMsg as character no-undo.
        define variable hNode as HANDLE no-undo.
        define variable memptrXml as memptr no-undo.
        define variable encoding as character no-undo initial 'ISO-8859'.
        CREATE X-NODEREF hNode.

        set-size(memptrXml) = 150.
        put-string(memptrXml, 1) = '<?xml version="1.0" encoding="' + encoding + '" standalone="no"?><references>"àáâãäå"</references>'.
        errMsg = "X-NODEREF or X-DOCUMENT LOAD got an error: FATAL ERROR: file 'MEMPTR', line '1', column '58'," +
                 "message 'unable to create converter for '" + encoding + "' encoding'. (9082)".

        xDocument:load("memptr", memptrXml, no).
        Assert:isFalse(lok).

        CATCH ex AS Progress.Lang.Error :
            Assert:Equals (1, ex:NumMessages).
            Assert:Equals (9082, ex:GetMessageNum(1)).
            Assert:Equals (errMsg, ex:GetMessage(1)).
        END CATCH.
    end method

 else if ("memptr".equals(loadMode))
      {
         ...
         try
         {
            memptr mptr = (memptr) source;
            byte[] bytes = mptr.getByteArray();
            ...
         }
         catch (IOException | ParserConfigurationException e)
         {
            invalidArgumentError("LOAD");
         }
         catch (SAXException e)
         {
            String msg = "X-NODEREF or X-DOCUMENT LOAD got an error: FATAL ERROR: " +
                         "file '%s', message '%s'";
            String err = String.format(msg, source, e.getMessage());

            commonDomError("LOAD", err);
         }

In 4GL we get the output: X-NODEREF or X-DOCUMENT LOAD got an error: FATAL ERROR: file 'MEMPTR', ..., but in FWD when catching a SAXException using a memptr, we get instead of 'MEMPTR' the content of the memptr variable.

Alexandru Lungu wrote:

PS: I think recordOrThrowError is more suitable. recordOrShowError will simply display the alert-box, but it won't throw any exception and will continue execution. The "record" term is used for cases when the statement is suffixed with NO-ERROR. In that case, 4GL will simply record the error in the error list, accessible in ERROR-STATUS system handle. You can check that. If the ERROR-STATUS is not updated in NO-ERROR mode, I suspect ErrorManager.throwError() is even more suitable (very improbable).

By replacing it with throwError or recordOrThrowError, the catch doesn't have 2 error codes. In 4gl it has error 9170 and warning 4065, but if I replace recordOrShow with recordOrThrow, it solves one tc but breaks another, this one:

    @Test.
    method public void LoadNoArgsNoSuppress ( ):
        define variable errMsg as character no-undo extent 2.
        xDocument:load("", "", no) no-error.
        assign
            errMsg[1] = 'Argument for LOAD/SAVE must be ~'file~', ~'stream~', ~'stream-handle~' or ~'memptr~'. (9170)'.
            errMsg[2] = '~*~*The LOAD attribute on the X-DOCUMENT widget has invalid arguments. (4065)'.
        Assert:Equals(2, error-status:NUM-MESSAGES).
        Assert:Equals(9170, error-status:GET-NUMBER(1)).
        Assert:Equals(4065, error-status:GET-NUMBER(2)).
        Assert:Equals(errMsg[1], error-status:GET-MESSAGE(1)).
        Assert:Equals(errMsg[2], error-status:GET-MESSAGE(2)).
    end method.

Discussed with Danut and we came to the conclusion to not assert the error messages if they are too complicated, only check if the correct error message is displayed and the number of exceptions thrown.
Regarding the space here:

loadNoArgsNoSuppress ✘ Expected: **The LOAD attribute on the X-DOCUMENT widget has invalid arguments. (4065) but was: ** The LOAD attribute on the X-DOCUMENT widget has invalid arguments. (4065)

It is not needed to change the buildErrorText method for this case specifically since there are many other errors that display correctly, so this message will not be checked.

Looking to run existing tests for SAXReader and stumbled upon AsertExt that is imported inside some test files. I tried adding to conversion but didn't manage to convert those tests.
Found this page https://proj.goldencode.com/projects/p2j/wiki/Testcase_Helper_Code but it seems it's not complete, now looking for a way to import this AssertExt.

     [java] Caused by: java.lang.NullPointerException: null value for annotation 'legacy-import':USING [KW_USING]:158913789969 @3:1
     [java]    support.test.AssertExt [SYMBOL]:158913789971 @3:7
     [java]    from [KW_FROM]:158913789973 @3:30
     [java]       propath [KW_PROPATH]:158913789975 @3:35
     [java] 

+Marian

   protected void commonDomError(String attribute, String message, boolean raiseError)
   {
      if (attribute == null)
      {
         throw new IllegalArgumentException("The attribute is null !");
      }

      String msg = "X-NODEREF or X-DOCUMENT %s got an error: %s";
      String err = String.format(msg, attribute.toUpperCase(), message);
      if (raiseError)
      {
         ErrorManager.recordOrThrowError(9082, err, false, false);
      }
      else
      {
         ErrorManager.recordOrShowError(9802, err, true, false, false);
      }
   }

I do have a question about this, shouldn't the else statement have 9082 aswell? This is inside XEntityImpl.commonDomError

Stefan Vieru wrote:

[...]

I do have a question about this, shouldn't the else statement have 9082 aswell? This is inside XEntityImpl.commonDomError

This is a typo, see trunk/13316 where this was added.

Stefan Vieru wrote:

Looking to run existing tests for SAXReader and stumbled upon AsertExt that is imported inside some test files. I tried adding to conversion but didn't manage to convert those tests.
Found this page https://proj.goldencode.com/projects/p2j/wiki/Testcase_Helper_Code but it seems it's not complete, now looking for a way to import this AssertExt.
[...]

Stefan, that is a known issue when converting 4GL code in FWD all classes/procedures used inside converted code must also be included in the file conversion file. Most of the time is just trial and error, add 'dependencies' until you get the conversion to pass :(

We used to try to figure out what the dependencies are and somehow improve the conversion, or use compile xref on Progress side to generate a more complete conversion file but for time being I guess manual maintenance of those conversion files is the only way to go.

During testing I have found that the converted code doesn't differentiate between a closed stream and a non-declared one.
Inside StreamWrapper constructor I added the remote stream to a hashmap.

   public StreamWrapper(String name)
   {
      super(name);
      work.get().addRemoteStream(name, this);
   }

Now the checking whether the stream exists but it's not open or is not declared:

         Stream sInstance = StreamWrapper.getRemoteStreamRef(sName);

         if (sInstance == null) 
         {
            // not declared
            String msg = "XML LOAD/SAVE could not find open stream " + sName;
            recordOrShowError(9169, msg, false, false, true);
         }
         else if (!sInstance.isValid()) 
         {
            // declared but not open
            String msg = "Attempt to read from closed stream " + sName;
            recordOrShowError(1386, msg, false, true, true);
         }
         else 
         {
            // open but this operation is not supported
            commonDomError("LOAD", "Not supported", false); 
         }

=== modified file 'src/com/goldencode/p2j/xml/XDocumentImpl.java'
@@ -776,13 +778,8 @@
-            String err = String.format(
-                  "FATAL ERROR: file '', line '0', column '0', message 'An exception occurred! " +
-                  "Type:RuntimeException, Message:The primary document entity could not be " +
-                  "opened. Id=%s'",
-                  fName);
-            commonDomError("LOAD", err);
-            
+            invalidArgumentError("LOAD");

Went through the tests in X-DOCUMENT for ENTITY-EXPANSION-LIMIT and added some to SAX-PARSER aswell.
Added the entityExpansionLimit variable to both SaxReaderImpl and XDocumentImpl.
Implemented the getters/setters and added setEntityExpansionLimit(integer limit) to XmlSchema interface that both classes implement.
The setter:

   @Override
   public void setEntityExpansionLimit(integer limit)
   {
      if (!limit.isUnknown() && limit.intValue() < 0)
      {
         String msg = String.format("Attribute ENTITY-EXPANSION-LIMIT must be greater than or equal to" +
                                    " 0 on %s widget", type());
         ErrorManager.recordOrShowError(4082, msg, false, true, false);
         return;
      }

      this.entityExpansionLimit = limit;
   }

Stefan Vieru wrote:

I have attached the patch with the changes regarding to SAX-PARSER.

Commit the changes to 9447a and put the issue in review if you are unsure of your approach or finished implementing the solution.

Had a double declaration in XDocument so the most up-to-date: 15717.

EntityExpansionLimit doesn't apply even if it's set.
I have found how to set a feature for DocumentBuilderFactory, using setFeature, but didn't find a parameter that corresponds with entity-expansion-limit.
In 4GL test setting this parameter has an impact when using load() for X-DOCUMENT.
For SAX-PARSER it doesn't work with the following testcase, maybe it's more permissive, i don't know. Meaning that setting the attribute doesn't have an impact when it should (case of X-DOCUMENT)

    @Test.
    method public void testSetHigherSource(  ):
        define variable hParser as handle no-undo.
        create sax-reader hParser.

        hParser:entity-expansion-limit = 1.
        hParser:set-input-source("FILE", "tests/base_language/xml/sax/support/test2.xml").
        hParser:SAX-PARSE() no-error.
        AssertExt:NotErrorNotWarning().

        finally:
            delete object hParser no-error.    
        end finally.
    end method.

Alex,
I have found this: https://docs.oracle.com/javase/tutorial/jaxp/limits/using.html
Setting the ENTITY-EXPANSION-LIMIT is done by using System.setProperty and i'm not sure it's ok. Could you please check this and also #9447-33.
Also found this: https://xerces.apache.org/xerces2-j/javadocs/api/javax/xml/validation/ValidatorHandler.html#setProperty(java.lang.String,%20java.lang.Object)

I think everything that is necessary is here. I still have the tests that I created locally, they're not on testcases, should I commit them?

Stefan Vieru wrote:

I still have the tests that I created locally, they're not on testcases, should I commit them?

Are they unit-tests?

They are the same format as the others in base_language/xml/sax/support, so I suppose yes.

Stefan Vieru wrote:

They are the same format as the others in base_language/xml/sax/support, so I suppose yes.

OK, commit them in a proper folder and post here the file names.

modified:
  tests/base_language/xml/sax/reader/attribute/TestEntityExpansionLimit.cls
added:
  tests/base_language/xml/dom/x_document/methods/TestI18n.cls
  tests/base_language/xml/dom/x_document/methods/TestI18nFile.cls
  tests/base_language/xml/dom/x_document/support/differentI18nEncoding.xml
  tests/base_language/xml/dom/x_document/support/i18nEncoding.xml
  tests/base_language/xml/sax/support/embeddedEntity.xml

revno: 1754

In order to work on this task, I need to create a new branch 9447b, or I can work on the existing branch, 9447a?

Use 9447a.

While building the FWD on this branch, I have encountered this error:

[ant:exec] /home/tg/gcd/branches/9447a/src/native/terminal_linux.c: In function ‘initConsole’:
[ant:exec] /home/tg/gcd/branches/9447a/src/native/terminal_linux.c:369:4: warning: implicit declaration of function ‘auto_getch_refresh’ [-Wimplicit-function-declaration]
[ant:exec]   369 |    auto_getch_refresh(FALSE); // disable sync during getch()
[ant:exec]       |    ^~~~~~~~~~~~~~~~~~
⋮
⋮
[ant:exec] /usr/bin/ld: terminal_linux.o: in function `initConsole':
[ant:exec] terminal_linux.c:(.text+0x60d): undefined reference to `auto_getch_refresh'
[ant:exec] collect2: error: ld returned 1 exit status
[ant:exec] make: *** [makefile:284: libp2j.so] Error 1

Teodor Gorghe wrote:

While building the FWD on this branch, I have encountered this error:
[...]
I'm using Ubuntu 24.04.2, with thread-safe patched libncurses 6.4, according to the wiki https://proj.goldencode.com/projects/p2j/wiki/Using_Thread-Safe_NCURSES_with_Static_Linking.
I have no issues building FWD from the trunk.

Looks like the function doesn't exist, are you sure you patched ncurses6.4 before building? Let me know if you used v6.1_20200708.patch patches available or the ones from #9042 (v6.4_20240113.patch).

I built with ./gradlew all and no issues on my part.

> Task :set-version-properties
[ant:echo] Using this version information (FWD v4.0.0_undefined_undefined_15717):
[ant:echo] Major: 4
[ant:echo] Minor: 0
[ant:echo] Release: 0
[ant:echo] Repo: undefined
[ant:echo] Branch: undefined
[ant:echo] Rev: 15717

> Task :ant-archive
> Task :archive
> Task :ant-all
> Task :all

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.6.4/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 3m 51s
47 actionable tasks: 47 executed

Dănuț Filimon wrote:

Teodor Gorghe wrote:

While building the FWD on this branch, I have encountered this error:
[...]
I'm using Ubuntu 24.04.2, with thread-safe patched libncurses 6.4, according to the wiki https://proj.goldencode.com/projects/p2j/wiki/Using_Thread-Safe_NCURSES_with_Static_Linking.
I have no issues building FWD from the trunk.

Looks like the function doesn't exist, are you sure you patched ncurses6.4 before building? Let me know if you used v6.1_20200708.patch patches available or the ones from #9042 (v6.4_20240113.patch).

I will look on it right now. I'm using the v6.4 patch. I have looked on it and this patch adds the auto_getch_refresh (bool) function. Maybe the setup_ncurses6x.sh from the trunk, modified, adding --disable-widec, does not apply the patch. I did run the script just with ./setun_ncurses6x.sh -f. I will try to run the scripts from the #9042.

Teodor Gorghe wrote:

I will look on it right now. I'm using the v6.4 patch. I have looked on it and this patch adds the auto_getch_refresh (bool) function. Maybe the setup_ncurses6x.sh from the trunk, modified, adding --disable-widec, does not apply the patch. I did run the script just with ./setun_ncurses6x.sh -f. I will try to run the scripts from the #9042.

Use the latest version of the script from trunk/tools/scrips/setup_ncurses6x.sh and modify it to use the right patches.

Dănuț Filimon wrote:

I will look on it right now. I'm using the v6.4 patch. I have looked on it and this patch adds the auto_getch_refresh (bool) function. Maybe the setup_ncurses6x.sh from the trunk, modified, adding --disable-widec, does not apply the patch. I did run the script just with ./setun_ncurses6x.sh -f. I will try to run the scripts from the #9042.

Use the latest version of the script from trunk/tools/scrips/setup_ncurses6x.sh and modify it to use the right patches.

When I have modified setup_ncurses6x.sh, I have commented the wget commands, and changed the patch filenames, but there are not located in the current directory. I did fix that when I did put ../ before the filename, because the patches are located in ncurses_static directory.

BUILD SUCCESSFUL in 3m 41s

Stefan Vieru wrote:

Alex,
I have found this: https://docs.oracle.com/javase/tutorial/jaxp/limits/using.html
Setting the ENTITY-EXPANSION-LIMIT is done by using System.setProperty and i'm not sure it's ok. Could you please check this and also #9447-33.
Also found this: https://xerces.apache.org/xerces2-j/javadocs/api/javax/xml/validation/ValidatorHandler.html#setProperty(java.lang.String,%20java.lang.Object)

I managed to set the entityExpansionLimit on DocumentBuilderFactory with factory.setAttribute("jdk.xml.entityExpansionLimit", "1000000"); (acording to the https://docs.oracle.com/en/java/javase/17/security/java-api-xml-processing-jaxp-security-guide.html). Also works with JDK 1.8. I will discuss with Constantin Asofiei, with a proper way to set this attribute (eg. creating a new XmlHelper.parse method or in getDocumentBuilderFactory method).

What about SaxReaderImpl?

For the SaxReaderImpl, it is even simplier. I think for the JDK 17, xmlif.setProperty("jdk.xml.entityExpansionLimit", entityExpansionLimit.getValue()); does the job. I need to investigate even further for JDK 1.8.

In order to be compatible with JDK 1.8, the code for setting entityExpansionLimit on factory instance is:

// Compatible with JDK 1.8 and JDK 17
// DocumentBuilderFactory
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setAttribute("http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit", "1000000");

// XMLInputFactory
XMLInputFactory factory = XMLInputFactory.newInstance();
factory.setProperty("http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit", 1000000);

Is there a constant in JRE for http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit?

Regarding to the X-DOCUMENT:LOAD method, for the longchar mode, on 4GL implementation, the encoding is not validated on this method, but validated using the X-DOCUMENT:SAVE method.
When using the memptr mode, on 4GL implementation, the encoding validation happens on X-DOCUMENT:LOAD.

I followed your suggestion provided yesterday, regarding using X-DOCUMENT:SAVE method when there is no encoding attribute in XML header. When saving the XML, it will add on the header, the default codepage, SESSION:CPINTERNAL, for my case. As it does on the X-DOCUMENT:LOAD method, it will use the default codepage as X-DOCUMENT:ENCODING, instead of the longchar variable codepage.

Regarding to the JASP property for setting the entityExpansionLimit, it is only available on xerces JDK implementation. FWD uses xercesImpl 2.1.12 from Maven Central.
I have commented the line 491 from build.gradle, but I still get xercesImpl.jar in the class path.
There is a way to completely remove the xercesImpl 2.1.12 dependency?

Run ./gradlew dependencies and look into the dependency tree. There is also #9687 where we are upgrading dependencies, but I don't see xerces in the task - check dependencies on 9687b.

Otherwise, X-DOCUMENT in FWD has direct dependencies on org.apache.xerces.dom..

Teodor Gorghe wrote:

Regarding to the X-DOCUMENT:LOAD method, for the longchar mode, on 4GL implementation, the encoding is not validated on this method, but validated using the X-DOCUMENT:SAVE method.
When using the memptr mode, on 4GL implementation, the encoding validation happens on X-DOCUMENT:LOAD.

I followed your suggestion provided yesterday, regarding using X-DOCUMENT:SAVE method when there is no encoding attribute in XML header. When saving the XML, it will add on the header, the default codepage, SESSION:CPINTERNAL, for my case. As it does on the X-DOCUMENT:LOAD method, it will use the default codepage as X-DOCUMENT:ENCODING, instead of the longchar variable codepage.

Please make sure to put code comments into these locations to explain these weird implementation choices of OE.

Greg Shah wrote:

Please make sure to put code comments into these locations to explain these weird implementation choices of OE.

Ok. I wrote some comments about that strange behavior of OE, in the X-DOCUMENT:LOAD implementation.

Constantin Asofiei wrote:

Run ./gradlew dependencies and look into the dependency tree. There is also #9687 where we are upgrading dependencies, but I don't see xerces in the task - check dependencies on 9687b.

Otherwise, X-DOCUMENT in FWD has direct dependencies on org.apache.xerces.dom..

dom4j, olap4j and gwt-dev have indirect dependency with xercesImpl.
Also, XDocumentImpl uses constants from org.apache.xerces for XML DOM Configuration and Java to IANA encoding mapping.
I need to make sure again, if xercesImpl has a entityExpansionLimit setting.
#9687 still has xercesImpl as dependency.

Checked the following files:
- src/com/sun/org/apache/xerces/internal/impl/Constants.java: internal JDK implementation, JDK_ENTITY_EXPANSION_LIMIT and JDK_ENTITY_EXPANSION_LIMIT exists. There are not accesible to the Java application, for security reason.
- src/com/sun/org/apache/xerces/impl/Constants.java: xerces 2.12.2, there is no JDK_ENTITY_EXPANSION_LIMIT or JDK_ENTITY_EXPANSION_LIMIT.

Currently, I am thinking about moving Constants.SCHEMA_NONS_LOCATION and Constants.DOM_SCHEMA_LOCATION to the new XmlProperty enum (with EntityExpansion constant), to remove references of xerces in XDocumentImpl. It also requires creating a new class, for mapping Java encoding names to the IANA encoding name.

Later edit:
- casting the Document to DocumentImpl is not possible because the class definition is within the internal XML package (not visible to the other Java classes). I think there are 2 solutions to that: use a javac argument to make the internal XML package available (which also makes EncodingMap available, not recommended for security reasons) or keep an external encoding variable and then no longer use document.getXmlEncoding() for obtaining the XML document encoding.

Commited revision 15718 on branch 9447a.
- removed olap4j dependency because is no longer needed
- changed xercesImpl with the internal xerces JDK implementation because it does not have support for changing the entityExpansionLimit property
- adjusted the code that directly used xercesImpl to be compatible with the JDK implementation.
- created a new class, EncodingMap, because the implemented class from xerces is not accesible.
- modified setEncoding method to no longer use the internal DocumentImpl class. With moving from xercesImpl to internal JDK, this class become inaccesible. On the save method, if the encoding was changed, transforms xdoc with the old encoding, to the new encoding. This is the behavior that also happens in OE.

Please review this revision and tell me what do I need to change. There is still more work to do, fixing the error handling in some cases.
The dependency dom4j, also uses xercesImpl. It is currently used in ColorMapLoader. I need to fully test that, to ensure that, changing from xercesImpl to internal xerces JDK implementation, does not create any bugs.

Teodor, are any of these changes still compatible with Java 8? Or 9447a will enforce a minimum support to Java 17?

I created a new Java 8 project, which I have tested this property, it works with no problems.
I am checking right now on p2j project.

The changes are compatible with Java 1.8.

╷
├─ JUnit Jupiter ✔
├─ JUnit Vintage ✔
├─ JUnit Platform Suite ✔
└─ FWD Test ✔
   └─ tests.base_language.xml.dom.x_document.attributes.TestEntityExpansionLimit ✔
      ├─ testGet ✔
      ├─ testGetValueWhenFileLoaded ✔
      ├─ testDefault ✔
      ├─ testSetNull ✔
      ├─ testSetNegativeValue ✔
      ├─ testSetZero ✔
      ├─ testSetMaxInt ✔
      ├─ testSetInt64 ✔
      ├─ testExpansionLimitLoad ✔
      └─ testExpansionDefaultLimitLoad ✔

Test run finished after 357 ms
[         5 containers found      ]
[         0 containers skipped    ]
[         5 containers started    ]
[         0 containers aborted    ]
[         5 containers successful ]
[         0 containers failed     ]
[        10 tests found           ]
[         0 tests skipped         ]
[        10 tests started         ]
[         0 tests aborted         ]
[        10 tests successful      ]
[         0 tests failed          ]
tg@tg-HP-Z230:~/gcd/testcases/deploy/client$ java -version
openjdk version "1.8.0_452" 
OpenJDK Runtime Environment (build 1.8.0_452-8u452-ga~us1-0ubuntu1~24.04-b09)
OpenJDK 64-Bit Server VM (build 25.452-b09, mixed mode)

Commited revision 15719 on branch 9447a.
- added a missed file, that must have been added on the previous commit
- implementation on Java 1.8 threw a different type of exception, at LOAD("longchar", ...) when providing XML with missing encoding. This change addresses a fix for this issue.

Tested ColorMapLoader. UI apps loads the correct colors and there seems to be no problems related with dom4j.

Commited revision 15720 in branch 9447a.
- fixed entityExpansionLimit on SAX-PARSER because StAX uses a different key name for this property.

╷
├─ JUnit Jupiter ✔
├─ JUnit Vintage ✔
├─ JUnit Platform Suite ✔
└─ FWD Test ✔
   └─ tests.base_language.xml.sax.reader.attribute.TestEntityExpansionLimit ✔
      ├─ testDefaultWithoutSource ✔
      ├─ testDefaultWithSource ✔
      ├─ testSetValid ✔
      ├─ testSetNegative ✔
      ├─ testSetUnknown ✔
      └─ testSetHigherSourse ✔

The -entityExpansionLimit should be at client part?
On 4GL, this is a startup parameter, like -cpintern.

Teodor Gorghe wrote:

The -entityExpansionLimit should be at client part?

Yes, if you want to set it at the bootstrap config, then it will be client:cmd-line-option:entityExpansionLimit for the FWD client. In directory.xml, would be in server/default/runtime/default/entityExpansionLimit.

Teodor, please rebase the branch - you need to do it on devsrv01.

Rebased task branch 9447a from P2J trunk revision 15955.
Commited revision 15963 on task branch 9447a:
- using logical constants instead of instances.

• SaxReaderImpl.java
  • remove '017' and replace it with spaces, each branch must have only a single history number for the changes in a file, for that branch.
  • entityExpansionLimit - javadoc must be on a single line
  • use integer.of instead of new integer for entityExpansionLimit and other places in this class. But why keep it as integer and not Integer Java?
  • setEntityExpansionLimit(integer limit) is missing javadoc
  • instead of this.entityExpansionLimit = limit; use this.entityExpansionLimit = integer.of(limit);. Remember, BDT instances are mutable (unless they are constants via i.e. integer.of(). But this is not needed if entityExpansionLimit is Java Integer - it can just be null for unknown. In any case, we want to reduce BDT usage from within FWD runtime, so use Java types wherever possible. Same as unneeded new integer to call just another overload.
• XCommonImpl.java
  • remove '012' and '013' texts from history entry
  • if you Revert 011 (recordOrShowError 4065 in some cases is a warning)., then just remove that history entry. What we care is the changes as they appear with the trunk revision for the branch.
• XDocumentImpl.java
  • remove '020' text from history entry
  • same changes about entityExpansionLimit as in SaxReaderImpl
  • buildXmlProperties - if -1 is the default, add a comment as to why is that OK.
  • why keep encoding as character and not String?
  • use character.of instead of new character() in this class
  • catch (SAXException | ParserConfigurationException e) needs to be aligned like this:
    

    catch (SAXException | 
           ParserConfigurationException e)
    

  • for "stream".equals(loadMode) || "stream-handle".equals(loadMode) - does 4GL not allow you to load an XML from a stream/-handle?
  • transform() - align the catch types vertically
• XmlProperty
  • toString() is missing javadoc
• EncodingMap.java - how did you build this class?
• XmlHelper.java
  • remove 036 text from the history entry
  • parse() - allign the parameters like:
    

       public static Document parse(InputStream              in,
                                    boolean                  validate,
                                    String                   systemId,
                                    boolean                  isNamespaceAware,
                                    Map<XmlProperty, Object> xmlProperties)
    

  • line 717 - remove the empty line above, we do not add empty line between param in javadoc
  • vertically allign the throws types in transform(), like:
    

       public static Document transform(Document source, String targetEncoding) 
       throws TransformerException,
              ParserConfigurationException,
              IOException,
              SAXException
    

  • transform() - why transform if there is no root element? You mean there can be CDATA/comments/etc, while a XML root node is missing?
  • please close the output/input streams
• build.gradle
  • history entry needs to be on a single line (max line length is 110)
  • line 495 and 504 need to be removed

Acording to the Progress ABL documentation, when entityExpansionLimit is set to unknown, there will be no limitation for the number of entity expansions. In order to achieve that in the Java XML parser, setting this property to -1 seems to disable this feature (I have tested in an empty Java project).
The code "stream".equals(loadMode) || "stream-handle".equals(loadMode) was introduced by Stefan Vieriu, when he worked on this project. I don't know how he discovered that, but it seems like an undocumented behavior from 4GL. Take a look on TestLoad.TestLoadStreamModeValidOpenedStream for more details. All tests passes on 4GL and FWD.

Constantin Asofiei wrote:

Review of 9447a rev 15963:

• XmlHelper.java
  • transform() - why transform if there is no root element? You mean there can be CDATA/comments/etc, while a XML root node is missing?
  • please close the output/input streams

This is a workaround to make XML parser to transform XML contents, like

<?xml version="1.0"?>

<?xml version="1.0" encoding="..."?>

Commited revision 15964 on task branch 9447a:
- It seems the XML transformation process now automatically resolves IANA encoding names to Java's internal charsets. I have removed the EncodingMap.java and the charset conversion is on Transformer class.
- Addressed code review of revision 15963.

I missed this in SaxReaderImpl - public void setEntityExpansionLimit(integer limit) - missing javadoc

Committed revision 15965 on branch 9447a.
- added javadoc on setEntityExpansionLimit(integer) from SaxReaderImpl.java

Teodor, please do run ETF with this.

Also, ask Stefanel, Lorian, Serban to help with the other apps.

Constantin Asofiei wrote:

Teodor, please do run ETF with this.

Also, ask Stefanel, Lorian, Serban to help with the other apps.

How is testing on this?

I have managed to test this on ETF, on last friday (check email).
I will ask right now to Stefanel, Serban and Lorian to test on other apps.

Did testing passed?

Lorian and Serban said that the tests have been passed. I found no issues on my tests.

Branch 9447a was merged to trunk rev 15982 and archived.

Do I need to merge now? I got the notification.

Not yet.

Please merge after 9456a.