Skip to content
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

openapi: add Application/XML support for request bodies #5751

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

trypa11
Copy link

@trypa11 trypa11 commented Sep 20, 2024

This commit adds support for handling Application/XML content types in request bodies for OpenAPI files

Overview

I added a new functionality that creates the XML body for the application/XML from OpenAPI files. Below is the output I received from ZAP after implementing the code.In this example, I modified the application/json schemas of the Crapi OpenAPI file to application/XML and achieved this outcome.
image

If you have any concerns or observations, please let me know.

Related Issues

zaproxy/zaproxy#6767

Checklist

  • Update help
  • Update changelog
  • Run ./gradlew spotlessApply for code formatting
  • Write tests
  • Check code coverage
  • Sign-off commits
  • Squash commits
  • Use a descriptive title

For more details, please refer to the developer rules and guidelines.

Copy link

github-actions bot commented Sep 20, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@trypa11
Copy link
Author

trypa11 commented Sep 20, 2024

I have read the CLA Document and I hereby sign the CLA

@trypa11

This comment has been minimized.

zapbot added a commit to zaproxy/cla that referenced this pull request Sep 20, 2024
Copy link
Member

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this modification handle nested objects?

generateXmlElements(property.getValue(), xml);
} else {
String value = dataGenerator.generateValue(elementName, property.getValue(), false);
if ("string".equals(property.getValue().getType())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

equalsIgnoreCase ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this function can handle nested objects because the generateXmlElements function utilizes recursion to create correct XML tags. Also, maybe it's better to use equalsIgnoreCase in this case. I didn't think of that before. Would you like to fix it, or should I make the changes with a new commit?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The could do an amending commit and force push

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for the advice and now you can review it again :)

This commit adds support for handling Application/XML content types in request bodies.This enhancement resolves Issue #6767

Signed-off-by:Tryfon Iason Papatriantafyllou <[email protected]>
@trypa11 trypa11 force-pushed the origin/openapiAppXml branch from c8d1a8c to 6d7c217 Compare September 21, 2024 08:16
@thc202
Copy link
Member

thc202 commented Sep 21, 2024

Was this tested with an actual application? Because I would have expected a root element to be present (even a dummy one), if we end up merging this we should be very clear that we are producing malformed XML document or that we produce only XML fragments.

@kingthorin
Copy link
Member

In the long run it may be better to use an actual XML Writer of some sort instead of 'manually' building the XML.

@thc202
Copy link
Member

thc202 commented Sep 23, 2024

Even in that case we will not be able to generate the correct payload as the OpenAPI spec does not always have all the info necessary to generate it.

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

Successfully merging this pull request may close these issues.

3 participants