Bug 219993 – $(findstring iphone,$(SDKROOT)) fails when SDKROOT is not lowercase

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Ryan Hostetler]

rdar://72436093

In our makefiles checks SDKROOT are case sensitive and fail if the SDKROOT has uppercase.
EG: $(findstring iphone,$(SDKROOT)) fails with SDKs cased as iPhoneOS.

This patch converts SDKROOT path to lowercase so any casing is matched in the same way.

[Ryan Hostetler]

Created attachment 416453 [details]
Patch

[Darin Adler]

Is there any user controlled directory name that can be in the SDKROOT path? What if the volume name contains the letter "iPhone", "TV", or "macOS" in them?

[Ryan Hostetler]

(In reply to Darin Adler from comment #2)
> Is there any user controlled directory name that can be in the SDKROOT path?
> What if the volume name contains the letter "iPhone", "TV", or "macOS" in
> them?

The default SDKROOT path lives within the Xcode directory eg:
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.0.sdk

The most likely problem would arise from a renamed Xcode eg: `Xcode-iphone.app`, which is still a risk without this patch.

If we wanted to protect further we could use match against `sdks/iphone`, though I'm not sure if there are use cases where the SDK would be moved out of the `SDKs` path.

[Darin Adler]

(In reply to Ryan Hostetler from comment #3)
> The default SDKROOT path lives within the Xcode directory eg:
> /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/
> Developer/SDKs/iPhoneOS13.0.sdk
> 
> The most likely problem would arise from a renamed Xcode eg:
> `Xcode-iphone.app`, which is still a risk without this patch.

Or an Xcode someone put into a directory like "/Users/darin/iPhoneTools".

And, yes, I was not suggesting it’s new in this patch, although doing case folding makes it even more likely to get it wrong.

> If we wanted to protect further we could use match against `sdks/iphone`,
> though I'm not sure if there are use cases where the SDK would be moved out
> of the `SDKs` path.

Sure, or even "platform/developer/sdks/iphone". Not sure what’s best.

[Darin Adler]

Or use something that extracts only the last piece of the SDKROOT path. There must be a function for that, maybe "basename".

[Ryan Hostetler]

(In reply to Darin Adler from comment #5)
> Or use something that extracts only the last piece of the SDKROOT path.
> There must be a function for that, maybe "basename".

Looks like notdir will grab just the SDK name:
$(notdir $(call to_lower,$(SDKROOT)))
> iphoneos13.0.sdk

[Ryan Hostetler]

Created attachment 416470 [details]
Patch

[Darin Adler]

Comment on attachment 416470 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416470&action=review

> Source/Makefile:6
> +	ifneq (,$(findstring macosx,$(notdir $(call to_lower,$(SDKROOT)))))

I think it would be better to call notdir first before calling shell, so we pass a shorter string. Not sure why we used all lowercase for "to_lower" when we use all uppercase for other make variables.

[Ryan Hostetler]

Created attachment 416476 [details]
Patch

[Ryan Haddad]

rdar://72436093

[EWS]

Committed r271037: <https://trac.webkit.org/changeset/271037>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416476 [details].