Re: RFR: 8250968: Symlinks attributes not preserved when using jarsigner on zip files

2020-09-06 Thread Seán Coffey

Thanks for the review Hai-May. I've implemented all your suggestions.

The CSR was approved late on Friday so I'll now submit this via PR on 
github infra.


regards,
Sean.

On 28/08/2020 21:08, Hai-May Chao wrote:
JarSigner.java #953: The output debug message can be removed from the 
code.

JavaUtilZipFileAccess.java #44: Change posixPerms to extraAttrs.
ZipFile.java #661: Suggest to keep the comment and update it with the 
additional 4 bits for symlink.


The rest of code changes and CSR look good.

Thanks,
Hai-May


On Aug 28, 2020, at 7:17 AM, Seán Coffey > wrote:


I've been poking around the zip internals and am now able to locate 
the 16 bits of interest. The position of these actual bits does 
appear to move around from one test run to another. For now, I guess 
it's sufficient to look for the pattern of interest in the signed zip 
file. New testcase added.


http://cr.openjdk.java.net/~coffeys/webrev.8250968.v4/webrev/

regards,
Sean.

On 27/08/2020 15:58, Weijun Wang wrote:

Looks like it was a conscious design decision to only allow recording of POSIX 
permission bits for this field (& 0xFFF). I don't see anything about symlink 
support in zipfs docs.

As long as that*byte*  is there and it’s not difficult to locate, we can 
manually add the*bit*  for symlink and see if jarsigner can keep it.

—Max





Re: RFR(S): 8252407: Build failure with gcc-8+ and asan

2020-09-06 Thread Florian Weimer
* Kim Barrett:

>> On Sep 4, 2020, at 7:50 AM, Florian Weimer  wrote:
>> 
>> * Daniel Fuchs:
>> 
>>> Hi,
>>> 
>>> On 02/09/2020 08:19, Florian Weimer wrote:
 At least one of the bugs was in theory user-visible: the network
 interface code would return data for an interface that does not actually
 exist on the system.
>>> 
>>> WRT NetworkInterface.c, I might support using `strnlen` to check
>>> the length before hand, if that solves both cases (gcc8 and gcc10).
>>> I'm always a bit nervous of changing the behavior in this library
>>> as it's hard to verify that no regression is introduced.
>> 
>> I think you should use strlen.  If the string is longer than the buffer
>> sent to the kernel, it cannot match an existing interface because all
>> the names are shorter.  So some sort of “not found” error needs to
>> reported.
>
> That may be, but I think doing so probably won't do anything to
> address the -Wstringop-truncation warnings.

There is no reason to use strncpy.  At least on Linux, the struct field
needs to be null-terminated, and you need to compute the length for the
length check.  So you might as well use memcpy with the length plus one
(to copy the null terminator).  You can keep using strncpy, and the
warning should be gone (because GCC will recognize a dominating strlen
check), but it's not necessary.

On current GNU/Linux, the most common structs now have the appropriate
annotations, so you get the strncpy warnings only in cases where there
is an actual truncation bug.

Thanks,
Florian