⏱ Estimated reading time: 5 minutes

Table of Contents

By Andoni Larzabal, PHP developer @ Conserto
Slides

The story starts with a project where a Zend component returned XML that needed to be updated by a Symfony app with one requirement: never using CDATA sections. CDATA allows escaping content that could be considered valid XML. In this case, the XML consumers did not support CDATA and adding support would require significantly more engineering effort than avoiding including them in the first place.

Symfony

The Symfony Serializer Component can serialize PHP objects to diverse data types such as XML, JSON and CSV. The XMLEncoder class forced adding CDATA sections when some string contained “<”, “>” or “&”. This is embarrassing given the requirement. This is problem as URL query parameters often contain “&” characters to denote the passing of more than one of them.

No decoration is possible: that method is private. Here are some alternatives:

  • Modifying the code in the dependencies directory: nope, it would be replaced on the next reinstall;
  • Suggesting the change in a GitHub issue on the Symfony project: that might take too long for anything to happen;
  • Hack the code using namespacing to circumvent the issue.

Indeed, the code used namespacing but invoked preg_match without the leading prefix “\” which meant that, by default, PHP looks for that function in the current namespace before defaulting to the runtime function of the same name. This overloading works but is risky: this hack can cause unexpected results from the uninformed user point of view, which in turn may lead to maintenance issues. Furthermore, it cannot be trusted: if the class code changes in just the wrong way, the hack stops working. No choice then: contributing to the core is inevitable.

First contribution

Symfony core contributions follow this procedure: find or create a GitHub issue, look for the contribution rules and read them, request that you are taking the fix, implement it and submit a Pull Request.

The implementation is where it starts to get interesting: you will need to be comfortable forking the project and working from a dedicated branch in your fork. Commit your changes to that branch, add tests and documentation, submit the Pull Request and then wait for the review. This first experience will introduce you to the contribution workflow and good code hygiene. Indeed, tests are a good habit. If you with to learn more about the Symfony community, visit the following page: https://symfony.com/community.

Once the Pull Request has been reviewed and accepted, expect an email stating the contribution has been merged to the core! Contributing truly is for everyone: there is no minimum lines of code or skills and the GitHub repository has plenty of good first issues to implement. If you have the time, go check these issues as most are about missing translations, documentation improvements, CI/CD work —GitHub Actions— or quirks that could help thousands of developers when fixed.

Contributions lead to other contributions: in the case of the preg_match invocation, a different contributor added support for custom regular expression patterns to cover even more scenarios.

Questions and Answers

How many lines of code are there in the XMLEncoder?

About 500 lines of terrible code written by many people over the years, generating some sort of incoherent lasagna code.

What is a good example of deep rework in Symfony?

The JSONEncoder class has seen significant performance improvements, internal cache use, has been rewritten from scratch and has been the occasion of several talks but is still officially in the talks because of how massively this class is being used and how impactful this change can be. Merging this code scares maintainers who will then need to assume responsibility.

Even if contributions like these do not get merged, then can lead to reworks that steer the framework in the right direction.

Are there other alternatives rather than contributing?

Well yes, you could patch vendors with Composer Patches and a Git diff file.

More about the missing “\” in preg_match

It was actually intended! The Symfony developers made it an unspoken rule to only prefix optimized functions with “\”. Overriding functions is therefore possible and allowed! This implies a malicious package could similarly “hack” core functions, so be careful and audit installed dependencies to check for similar hacks in them.

What is the policy regarding tests?

They are required whenever code is involved, i.e. for features and fixes. Develop the right habits: write tests stating your expectations before the contribution. If you need help, ask the community for advice.