-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Codec errors with latest NetWeaver release #196
Comments
@boringDev hey I am having the same issue but I don't have PL6 sdk. Can you share that SDK with me for Ubuntu ?? |
Hi @apoorv28goel , you can find PL6 here: |
@boringDev I c
hi @boringDev I can't find any download link on this page. Can you share the sdk files with me directly ?? |
Encoding errors are reproduced with PL7, on Darwin and Linux platform and the investigation is ongoing. Using PL6 is recommended for now. |
Root cause found, patch will be available soon. |
Please try the 2.1.0 release |
Thanks for the quick turnaround @bsrdjan! I can confirm that the 2.1.0 release is effective for my use-case. |
@bsrdjan: Can you maybe tell me the root cause of the problem and how you've fixed it? I'm having problems with the PL7 as well in our custom C-Wrapper.. From my analysis (valgrind) it looks like the |
Hi @hansingt and thank you for the input. It helps further investigate the root cause. The PyRFC used the https://github.com/SAP/PyRFC/blob/master/src/pyrfc/_pyrfc.pyx#L2398 I did not catch issues with |
If |
I've created a (kind of) minimal example, which can be used to observe the problem with the PL7: https://gist.github.com/hansingt/0f51696a8f9f10243b3419465d64b6d0 When compiling using the default flags from OSS-Note 2573953 and analyzing it using valgrind: One one observe (beside some unrelated uninitialized values) one additional error with PL:
Which isn't reported, when running it using PL6. Maybe, we need to report the issue to SAP Guys? How can this be done? |
Hi hansingt, the overrun in RfcSAPUCToUTF8() has been confirmed over here and fixed for PL 8. To be more precise: This also indicates the workaround you can use until release of PL 8: make sure that the output buffer passed into RfcSAPUCToUTF8() is > twice the necessary size... And then zero-terminate yourself at index *resultLen. (When converting to UTF-8, a bigger output buffer is useful anyway, because only ASCII characters are converted to 1 UTF-8 byte. ISO-Latin-1 characters like "ä" are converted to two bytes, Chinese & Japanese to 3 or even 4 bytes, and the "worst case" that can happen is that 1 Unicode character gets converted to 5 UTF-8 bytes. This may in fact be the reason why a simple error like this has not been caught earlier: in order to be on the safe side, most programs use the "rule of thumb" utf8-size = 5 x numUnicodeChars ...) Best Regards, Ulrich |
@hansingt, are you using PyRFC or your own C++ wrapper of NWRFC SDK ? You can also open SAP support ticket for NWRFC SDK component directly, if you want. Just in case you need more support with NWRFC SDK. |
@bsrdjan We are currently using our own C-Wrapper for Python, as we still need to support Python 2.7 (sadly 😢). @Ulrich-Schmidt Thanks for the information! Using your workaround fixes the problem for us till there is a patch available. |
This workaround should help with PL7: #213 |
The release 2.3.0 with the fix is now on PyPI, production stable tested. The documentation will be updated in the next PyRFC release, planned after SDK PL8 release. |
SAP released PL 7 for NetWeaver RFC SDK 7.5 on September 30th, 2020. This version of the SDK seems to be incompatible with the latest PyRFC 2.0.6 version. Here is a simple replication:
With PL 6:
We have been using PL 6 on various Windows instances without issue since it was released.
With PL 7:
I have been able to reproduce this on several versions of Windows Desktop/Server, Debian 10, and Darwin 18. The issue also appears to occur on all RFCs that I tested.
Any thoughts on the cause? Is support for PL 7 in development or am I missing a configuration in which 2.0.6 would support this new release?
The text was updated successfully, but these errors were encountered: