Dangerous Logging in Swift | Hacker News Reader

Dangerous Logging in Swift

indiestack.com

61 pointsingve25 comments

Always use a string literal as the first argument to NSLog or other printf type functions, but I don’t think the author correctly identified the cause of the crash. I believe floating point arguments are passed in registers, so you’ll just get whatever happens to be there. Besides, just reading value arguments like that should read junk from your stack, not a seg fault. Maybe he redacted the actual argument? An accidental %s or %n could lead to this behavior as it interprets junk as a pointer that is then accessed.

Indeed the intent, though not expressed in the Objective-C declaration through the type system, is that the format string is a string literal.

However the intent of the Swift call described in the article is indeed that of a string literal with 2 data "parameters".

So is the correct use of NSLog in this case is this:

    NSLog("%@", "Failed to get modification date at URL: \(url) - \(error)")

This should really be the behavior of NSLog in Swift, that it expands not to the variadic

    NSLog(format-string-parameter, objc-object-parameters..)

but to

    NSLog(@"%@", string-parameter)

Correct would be

NSLog(“Failed to get modification date at URL: %@ - %@“, url.absoluteString, error.localizedDescription)

Swift explicitly knows the difference between this construction and an actual string literal. Its type system considers "Foo" to be a StaticString, while "Foo \(something)" is merely a String, because it will need runtime allocation to calculate the actual value. A StaticString is actually a cheaper representation as well as needing to be distinguished for this reason.

So a native Swift API would be able to distinguish and refuse to let you provide this String where a StaticString is the appropriate thing, I am not an Apple expert to say whether this would be possible / easy for the NSLog binding, if it was possible to do this it should have been done.

NSLog takes dynamic strings just fine. Arbitrarily constraining it to only accept StaticStrings would break valid Swift programs. I wouldn't necessarily be opposed to it, but it's certainly not something that can be done without thoughtful consideration of the side effects.

In general, it is valid to resolve localized strings for use as format strings, which means that it is valid to have dynamic format strings at runtime.

NSLog might be considered an exception vs other Objective-C usage of format strings - but it might also not be worth having a singular special case.

Apple's va_args ABI passes arguments on the stack. The actual argument works, but in the actual crashing case the URL was actually printed twice (the crasher was something like NSLog("\(url) \(error)"), the latter of which contained the error string again.)

He didn't speak to why he's using NSLog. There are better alternatives that player nicer with Swift. https://developer.apple.com/documentation/os/logging

The article implies that the code was ported from objective-c to swift.

> A few years ago Apple introduced a new logging framework called the Unified Logging System, which supports logging with variadic arguments from both Swift and Objective-C. It also supports a host of other features that allow you to categorize and prioritize your log message and, well, it sounds great, but it’s also quite a bit more complicated than just invoking NSLog. It’s complicated enough that I can’t offer a one-line fix for the situation I’ve described here, but I encourage you to investigate your options.

Unified logs are an overkill API that isn't particularly well-suited for general purpose use like NSLog is. It's really more of something Apple uses themselves that they decided to throw over the wall for third party developers to adopt, but very few have done so due to its strange interface and goals that don't match what most app developers want.

This is Foundation API, not Swift stdlib. One thing Swift probably can do, is to have the first parameter typed as StaticString. I am not sure if the header for NSLog has enough annotations to do so.

OTOH: https://github.com/apple/swift-log

Yes, format strings should be required to be static strings. Logging frameworks, printf, i18n frameworks, anything that has arbitrary format strings they need to be literals or you're going to shoot yourself in the foot.

I'm surprised here that the author ends up believing that, on the one hand, this function behaves as if it has variadic parameters (hence the format specifiers), but then on the other hand they have no option but to somehow write all their text as the first parameter. These are logically inconsistent beliefs, but maybe too much fighting with Apple's ecosystem causes you to just stop believing the world has to make sense.

My experience is that NSLog is suprisingly slow. I remember investigating why a program was running so slowly and removing NSLog from the main loop made it significantly faster. Maybe the underlying console was slower than I expected, or the messages had to be sent through my iPhone's USB?

Because of this and the author's issue, I don't see why you should ever use NSLog in Swift. Even if you need a timestamp, you could just write a custom logging function using printf.

NSLog goes to the system console; printf does not.

I’ve just used print since going to swift from objective c. What’s the advantage of still using NSLog?

NSLog does insert a timestamp into the log message, which can be useful.

Good detective work.

I always use print(), which has its own issues.

Just curious, why not use https://developer.apple.com/documentation/os/logging ?

Mostly, because that's overkill. print() is very simple.

On the face of it, it looks like a potential source for security issues when untrusted input is logged.

No matter how many new languages we invent, this problem of surprise sub-languages being parsed from strings never goes away. It's almost as if program code should be somehow impossible to confuse with user-visible text. Maybe there's no good solution though :(

The appropriate constraint is well known and exists in several languages. The magic format strings mustn't be variables. In Swift there's a type StaticString which matches literals like "This is some text" but is not used for any interpolated strings, or other arbitrary variable values.

Of course just because the constraint is known doesn't mean (as you illustrate) that everybody knows about it or makes use of it in their own programming.

This isn't an "appropriate constraint", because dynamically constructing format strings is a thing that normal programs can and do do. StaticString exists due to various limitations on the os_log APIs and it's somewhat amusing that you're trying to spin it into some sort of "known" fact that everyone should follow.

In almost all cases this feature is a foot gun, reminiscent of eval() and similar cases and so should not be provided. Sure, it's easy and flexible, but in exchange it's horribly unsafe.

Imagine that, expecting a RESTful API for some new remote service you're accessing you discover it instead offers precise instructions for where the Javascript buttons are rendered on a 640x480 Internet Explorer 6 browser, instructing you to automate pressing the buttons through a browser automation gadget.

You'd be aghast right? Sure this would work but it's clearly not an ergonomic way to provide automation, why not just offer a RESTful API like most similar sites?

Dynamically constructing format strings is the same ergonomic awkwardness, for the benefit of lazy workers who couldn't be bothered to actually deliver what was needed.

Instead I want a way to reach into the same formatting infrastructure that is used for my string literal formats, and manipulate it dynamically, not by creating "format strings" dynamically.

As an example, if your formatting library can format("{some}{values}{with:parameters}", v1, v2, v3) I ought to be able to write my own different function myfn("[different][syntax][same#features]", v1, v2, v3) re-using the infrastructure from the existing formatting library, rather than needing to write a function myfn("[different][syntax][same#features]") which has a result "{some}{values}{with:parameters}" in order to deliver the same effect but via this sub-language.

Reply on news.ycombinator.com