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

Revert "libatalk: Restore invalid metadata cleanup in ad_open.c" #997

Closed
wants to merge 2 commits into from

Conversation

rdmark
Copy link
Member

@rdmark rdmark commented May 18, 2024

Reverts #513

@rdmark rdmark requested a review from dgsga as a code owner May 18, 2024 07:07
@rdmark
Copy link
Member Author

rdmark commented May 18, 2024

@dgsga All of a sudden we have multiple reports that the EA metadata validation still fails in some environments.

#992
#993

I propose we revert this for now to buy us some time to get to the root of it, without blocking the 3.2.0 release. Thoughts?

Copy link

sonarcloud bot commented May 18, 2024

@rdmark rdmark marked this pull request as draft May 18, 2024 10:09
@rdmark
Copy link
Member Author

rdmark commented May 18, 2024

Some additional investigation needed first...

@dgsga
Copy link
Contributor

dgsga commented May 18, 2024

@rdmark, I don't have any issue with this PR but IMHO the CVE regression fixes that were pushed to main have never been as simple and clean as @andychen-syno's original PR that was never accepted:

9e864d1

I use it on my fork and it produces a clean log with Mac OS 9 clients (unlike current main), none of these invalid metadata errors, and also allows for icon substitution for the AFP share as it should with this client OS. @andychen-syno subsequently force-pushed a mod to this commit that allowed it to be merged after @anodos325's commits were merged with main.

@rdmark
Copy link
Member Author

rdmark commented May 18, 2024

@dgsga I'm open to suggestions!

In main we have both Andrew's and Andy's patches, the latter applied on top of the former.

The former was merged with #174 and the latter with #178

I'm all for revisiting the solution, since we're still facing this apparent fragility in that area...

@dgsga
Copy link
Contributor

dgsga commented May 18, 2024

I'll prepare a draft PR...
UPDATE: PR #1007

@rdmark
Copy link
Member Author

rdmark commented Jun 2, 2024

When working on the 3.2.0 release notes, I was reminded that we have a fix for macOS generated EA metadata with #575 – closing this PR for now while evaluating if this is enough to resolve users' issues.

@rdmark rdmark closed this Jun 2, 2024
@rdmark rdmark deleted the revert-513-rdmark-issue-400 branch June 2, 2024 07:58
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.

None yet

2 participants