SAP / PyRFC

Asynchronous, non-blocking SAP NW RFC SDK bindings for Python
http://sap.github.io/PyRFC
Apache License 2.0
510 stars 137 forks source link

Codec errors with latest NetWeaver release #196

Closed boringDev closed 4 years ago

boringDev commented 4 years ago

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:

Python 3.5.4 (v3.5.4:3f56838, Aug  8 2017, 02:17:05) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> patch_level = 'PL6'
>>> from json import loads
>>> from os import environ
>>> conn_kwargs = loads(environ['CONN_KWARGS'])
>>> environ['SAPNWRFC_HOME'] = 'C:\\nwrfcsdk_750_{}'.format(patch_level)
>>> environ['PATH'] += ';C:\\nwrfcsdk_750_{}\lib'.format(patch_level)
>>> import pyrfc
>>> pyrfc.__version__
'2.0.6'
>>> len(pyrfc.Connection(**conn_kwargs).call('DDIF_FIELDINFO_GET', TABNAME='T001'))
6

We have been using PL 6 on various Windows instances without issue since it was released.

With PL 7:

Python 3.5.4 (v3.5.4:3f56838, Aug  8 2017, 02:17:05) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> patch_level = 'PL7'
>>> from json import loads
>>> from os import environ
>>> conn_kwargs = loads(environ['CONN_KWARGS'])
>>> environ['SAPNWRFC_HOME'] = 'C:\\nwrfcsdk_750_{}'.format(patch_level)
>>> environ['PATH'] += ';C:\\nwrfcsdk_750_{}\lib'.format(patch_level)
>>> import pyrfc
>>> pyrfc.__version__
'2.0.6'
>>> len(pyrfc.Connection(**conn_kwargs).call('DDIF_FIELDINFO_GET', TABNAME='T001'))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "src/pyrfc/_pyrfc.pyx", line 438, in pyrfc.pyrfc.Connection.call
  File "src/pyrfc/_pyrfc.pyx", line 2053, in pyrfc.pyrfc.wrapResult
  File "src/pyrfc/_pyrfc.pyx", line 2127, in pyrfc.pyrfc.wrapVariable
  File "src/pyrfc/_pyrfc.pyx", line 2071, in pyrfc.pyrfc.wrapStructure
  File "src/pyrfc/_pyrfc.pyx", line 2139, in pyrfc.pyrfc.wrapVariable
  File "src/pyrfc/_pyrfc.pyx", line 2329, in pyrfc.pyrfc.wrapString
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x96 in position 1: invalid start byte

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?

apoorv28goel commented 4 years ago

@boringDev hey I am having the same issue but I don't have PL6 sdk. Can you share that SDK with me for Ubuntu ??

boringDev commented 4 years ago

Hi @apoorv28goel , you can find PL6 here: https://launchpad.support.sap.com/#/notes/2840214

apoorv28goel commented 4 years ago

@boringDev I c

Hi @apoorv28goel , you can find PL6 here: https://launchpad.support.sap.com/#/notes/2840214

hi @boringDev I can't find any download link on this page. Can you share the sdk files with me directly ??

bsrdjan commented 4 years ago

Encoding errors are reproduced with PL7, on Darwin and Linux platform and the investigation is ongoing. Using PL6 is recommended for now.

bsrdjan commented 4 years ago

Root cause found, patch will be available soon.

bsrdjan commented 4 years ago

Please try the 2.1.0 release

boringDev commented 4 years ago

Thanks for the quick turnaround @bsrdjan! I can confirm that the 2.1.0 release is effective for my use-case.

hansingt commented 4 years ago

@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 RfcUTF8ToSAPUC function is writing beyond the buffer boundaries, which corrupts the heap..

bsrdjan commented 4 years ago

Hi @hansingt and thank you for the input. It helps further investigate the root cause. The PyRFC used the strlen() to right trim the RfcSAPUCToUTF8 output. With PL7 the output was corrupted with non-zero trailing characters and now trimmed using the correct length of the string, resultLen, determined by RfcSAPUCToUTF8:

https://github.com/SAP/PyRFC/blob/master/src/pyrfc/_pyrfc.pyx#L2398

I did not catch issues with RfcUTF8ToSAPUC but will check, thanks!

hansingt commented 4 years ago

If RfcSAPUCToUTF8 and RfcUTF8ToSAPUC are lacking the terminating NULL byte, this makes my impression that the PL7 oder the libs are bugged even stronger. If the Input string has a trailing NULL byte, the output should have one as well.

hansingt commented 4 years ago

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: valgrind --leak-check=full sapnwrfc_test

One one observe (beside some unrelated uninitialized values) one additional error with PL:

Invalid read of size 1
   at 0x4C80C14: RfcConverter::sapucToutf8(char16_t const*, char16_t const*, unsigned int, unsigned char*, unsigned int*, unsigned int*, unsigned int) (in [...]/libsapnwrfc.so)
   by 0x4C1C368: RfcSAPUCToUTF8 (in [...]/libsapnwrfc.so)
   by 0x109492: convertSAPUCToUTF8 (sapnwrfc_test.c:20)
   by 0x10925A: main (sapnwrfc_test.c:88)
 Address 0x5e31807 is 23 bytes after a block of size 32 in arena "client"

Which isn't reported, when running it using PL6. Maybe, we need to report the issue to SAP Guys? How can this be done?

Ulrich-Schmidt commented 4 years ago

Hi hansingt,

the overrun in RfcSAPUCToUTF8() has been confirmed over here and fixed for PL 8. To be more precise: if the lenght of the resulting UTF-8 string is n, then instead of writing the terminating zero at position n+1, RfcSAPUCToUTF8() writes it at position 2n+1 by mistake... :-(

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

bsrdjan commented 4 years ago

@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.

hansingt commented 4 years ago

@bsrdjan We are currently using our own C-Wrapper for Python, as we still need to support Python 2.7 (sadly :cry:).

@Ulrich-Schmidt Thanks for the information! Using your workaround fixes the problem for us till there is a patch available.

bsrdjan commented 3 years ago

This workaround should help with PL7: #213

bsrdjan commented 3 years ago

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.