Do we need to call multipart.Part.Close

[Pedro Luis Guzmán Hernández]

multipart.Part, returned by multipart.Reader's NextPart method, have a Close() method. The only example here https://pkg.go.dev/mime/multipart#NewReader doesn't use the Close() method at all, so what's it purpose? Can we safely ignore it?

The reason I'm asking is that, calling defer part.Closer is a bit annoying when you loop through a Reader (see the example mentioned above). Calling the defer within the loop means all parts are closed at the end of the function. The alternative would be to have an anonymous function within the loop and call defer within it, but it feels so awkward.

[Brian Candler]

https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/mime/multipart/multipart.go;l=325

All it does is read all the remainder of the part to io.Discard.  So if you're sure you've read each part before moving onto the next one, it looks like you should be good.

[Pedro Luis Guzmán Hernández]

Thanks Brian. That is an implementation detail though, so relying on it with no mention in the documentation at all feels unsound. A Close method usually means you have to defer it right after getting the resource, so I would have expected the docs to be more clarifying on its usage.

[Brian Candler]

Documentation could certainly be improved, since the Part.Close() method has literally no documentation.

Whilst it does feel unsound, in practice I don't think the behaviour could be changed now without breaking the Go compatibility guarantee (unless it was fundamental to a security issue, and even then I'd expect it to allow existing use cases).

[Ian Lance Taylor]

On Tue, Feb 6, 2024 at 6:29 AM Pedro Luis Guzmán Hernández
<pete...@gmail.com> wrote:
>
> Thanks Brian. That is an implementation detail though, so relying on it with no mention in the documentation at all feels unsound. A Close method usually means you have to defer it right after getting the resource, so I would have expected the docs to be more clarifying on its usage.

Yeah, the Close method doesn't seem to be documented at all. It would
be nice if somebody sent a patch. Looks to me like you don't ever
have to call Close, and that it should probably not have been
exported. It is called by Reader.nextPart, and I don't immediately
see any reason for it to be called anywhere else.

Ian

> El martes, 6 de febrero de 2024 a las 15:11:55 UTC+1, Brian Candler escribió:
>>
>> https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/mime/multipart/multipart.go;l=325
>>
>> All it does is read all the remainder of the part to io.Discard. So if you're sure you've read each part before moving onto the next one, it looks like you should be good.
>>
>> On Tuesday 6 February 2024 at 13:34:16 UTC Pedro Luis Guzmán Hernández wrote:
>>>
>>> multipart.Part, returned by multipart.Reader's NextPart method, have a Close() method. The only example here https://pkg.go.dev/mime/multipart#NewReader doesn't use the Close() method at all, so what's it purpose? Can we safely ignore it?
>>>
>>> The reason I'm asking is that, calling defer part.Closer is a bit annoying when you loop through a Reader (see the example mentioned above). Calling the defer within the loop means all parts are closed at the end of the function. The alternative would be to have an anonymous function within the loop and call defer within it, but it feels so awkward.
>

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/6371b366-8474-46f0-99bc-510471fb879bn%40googlegroups.com.

[Pedro Luis Guzmán Hernández]

Thanks all,

I'll continue to call it for now, since that's what most engineers on my team would expect in that situation, but I'll happily reconsider if some documentation gets added at some point.