Skip to content

Adding custom envelope key option for server and client header fix (take 2 for 1208 and 1170) #1330

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

smokhov
Copy link
Contributor

@smokhov smokhov commented Aug 9, 2025

This PR supersedes #1208 (by @gamzeoguz) by making sure the tests pass, adding 2 new tests and covering Faults. The original PR's 2 tests were not passing because actually they were Faults in response as empty Body is not allowed at the server by default here:

if (!body) {
Thus. it was failing to parse. I've converted those to Fault tests instead with and without envelopeKey set to soapenv. To cover happy-case scenarios I've added two more tests with some actual Body and expected response from other tests, with and without envelopeKey set to soapenv at for the client with some request data. So this covers #1208.

For #1170 (by @amalashenok) at first I though it was also relevant as the line was around the envelopeKey handling for the client, but it turns out it was not the case, so I wrote a brand new test for it after reproduction. Please check.

@smokhov smokhov marked this pull request as draft August 9, 2025 00:39
@smokhov
Copy link
Contributor Author

smokhov commented Aug 9, 2025

Current coverage:

File % Stmts % Branch % Funcs % Lines Uncovered Line #s
All files 94.3 90.77 92.26 94.4
node-soap 100 100 100 100
index.js 100 100 100 100
node-soap/src 92.8 85.99 91.15 92.74
client.ts 92.05 87.01 83.72 92.3 ...,423,454,496-500,569-572
http.ts 92.17 83.87 100 92.17 145-148,195-201,227,231,236
nscontext.ts 86.36 82.5 100 86.36 ...,51-57,84-86,138,161,229
server.ts 92.43 85.98 100 92.36 ...,437,473,517,523,548-558
soap.ts 97.53 91.3 76 97.01 45-46
utils.ts 100 85.71 100 100 10-12
node-soap/src/security 96.42 96.21 88.67 96.97
BasicAuthSecurity.ts 88.88 50 75 88.88 22
BearerSecurity.ts 87.5 100 75 87.5 20
ClientSSLSecurity.ts 100 100 100 100
ClientSSLSecurityPFX.ts 100 100 100 100
NTLMSecurity.ts 88.88 100 75 88.88 29
WSSecurity.ts 97.61 94.44 100 97.61 103
WSSecurityCert.ts 98.9 97.36 93.75 100 125
WSSecurityCertWithToken.ts 100 96 100 100 105
WSSecurityPlusCert.ts 14.28 100 0 16.66 6-13
index.ts 100 100 100 100
node-soap/src/wsdl 94.8 92.6 95.12 94.92
elements.ts 97.84 96.62 97.29 98 ...-414,431,567,581,629,923
index.ts 92.55 90.58 91.83 92.66 ...1391-1392,1427-1430,1435

@smokhov smokhov marked this pull request as ready for review August 10, 2025 04:53
@smokhov
Copy link
Contributor Author

smokhov commented Aug 10, 2025

Did a final test for #1170 so over to you @w666

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants