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

Allow PemContent.of to work with untrimmed content #41540

Closed
yingzhuo opened this issue Jul 17, 2024 · 9 comments
Closed

Allow PemContent.of to work with untrimmed content #41540

yingzhuo opened this issue Jul 17, 2024 · 9 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@yingzhuo
Copy link

Hello.
I am currently using Spring Boot 3.3.1, I noticed the utility org.springframework.boot.ssl.pem.PemContent is very useful.

how about add a PropertyEditor to the package? here is my code:

package org.springframework.boot.ssl.pem;

import org.springframework.boot.ssl.pem.PemContent;
import org.springframework.util.StringUtils;

import java.beans.PropertyEditorSupport;
import java.util.Arrays;
import java.util.stream.Collectors;

/**
 * @author Ying Zhuo
 */
public class PemContentEditor extends PropertyEditorSupport {

    /**
     * {@inheritDoc}
     */
    @Override
    public final void setAsText(String text) throws IllegalArgumentException {
        try {
            super.setValue(PemContent.of(trimContent(text)));
        } catch (Exception e) {
            throw new IllegalArgumentException(e.getMessage(), e);
        }
    }

    private String trimContent(String text) {
        return Arrays.stream(StringUtils.delimitedListToStringArray(text, "\n"))
                .map(String::trim)
                .collect(Collectors.joining("\n"));
    }
}

so i can config my bean like this:

<bean id="jwtSigner" class="com.mycompany.myproject.jwt.signer.AsymmetricSignerFactoryBean">
    <property name="certificateContent">
        <value>
            <![CDATA[
            -----BEGIN CERTIFICATE-----
            x509 format content here
            -----END CERTIFICATE-----
            ]]>
        </value>
    </property>
    <property name="keyContent">
        <value>
            <![CDATA[
            -----BEGIN PRIVATE KEY-----
            PKCS#8 format content here
            -----END PRIVATE KEY-----
            ]]>
        </value>
    </property>
    <property name="keyPassword">
        <null/>
    </property>
</bean>

@scottfrederick your code of SSL is very cool, thanks.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 17, 2024
@philwebb
Copy link
Member

I'm not to keen to add a PropertyEditor, but a Converter might be an option. I wonder if ObjectToObjectConverter might work out-of-the-box if we refined the of method to deal with trimming strings.

@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 22, 2024
@philwebb philwebb changed the title little suggestion about ssl util PemContent Add converter or property editor support so PemContent can be used in beans XML Jul 22, 2024
@philwebb philwebb changed the title Add converter or property editor support so PemContent can be used in beans XML Allow PemContent.of to work with untrimmed content Jul 24, 2024
@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Jul 24, 2024
@philwebb philwebb added this to the 3.4.x milestone Jul 24, 2024
@philwebb
Copy link
Member

We discussed this today and we don't think we should add the property editor, but we do think we can make the of() method do the trimming.

@yingzhuo
Copy link
Author

We discussed this today and we don't think we should add the property editor, but we do think we can make the of() method do the trimming.

Thank you, sir. I am so glad to hear that.

@wilkinsona
Copy link
Member

If we refine only of, I wonder if isPresentInText(String) and the various load methods behaving differently will cause confusion. It feels like trimming should either be done consistently or it should only be done by a method that makes it clear that its behavior is different. Perhaps of could be that method but it feels a bit hidden and might be better if the method name made the behavior clear.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Aug 1, 2024
@yingzhuo
Copy link
Author

yingzhuo commented Aug 2, 2024

Perhaps of could be that method but it feels a bit hidden and might be better if the method name made the behavior clear.

just a hint, sir.

ObjectToObjectConverter can only find static method valueOf() or of(). if method name be 'ofTrimmedString()'
the converter cannot work.

@mhalbritter
Copy link
Contributor

mhalbritter commented Aug 8, 2024

Is the trimming necessary? the getCertificates and getPrivateKey methods also work with un-trimmed content (they ignore everything before the --- BEGIN header and the footer and don't care about whitespace in between).

The only way to get the original un-trimmed content back is by calling toString.

So maybe just adding a converter without the trimming is sufficient?

@yingzhuo did i miss something? Is the trimming necessary?

@yingzhuo
Copy link
Author

yingzhuo commented Aug 9, 2024

@mhalbritter Thank you. sir
my suggestion is: trim each line of content, including footer and header.

/*
TEST NG
java.lang.IllegalStateException: Missing certificates or unrecognized format

	at org.springframework.util.Assert.state(Assert.java:76)
	at org.springframework.boot.ssl.pem.PemCertificateParser.parse(PemCertificateParser.java:64)
	at org.springframework.boot.ssl.pem.PemContent.getCertificates(PemContent.java:64)
	at com.github.yingzhuo.playground.PemContentTestCase.test1(PemContentTestCase.java:22)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
*/
@Test
void test1() {
    var pem = """
                        -----BEGIN CERTIFICATE-----
                        MIHrMIGeoAMCAQICFF4UkhlALvGwUHfuZBulAZDim5rlMAUGAytlcDAVMRMwEQYD
                        VQQDDApwbGF5Z3JvdW5kMCAXDTIzMTIzMTE2MDAwMFoYDzIxMjQxMjMxMTU1OTU5
                        WjAVMRMwEQYDVQQDDApwbGF5Z3JvdW5kMCowBQYDK2VwAyEAl1mgT4NWO98CmI8L
                        BfKUV7TU9OQUdBDEOqgLAZyPzQkwBQYDK2VwA0EAm+Xj5Zhqk3tcSWEKEnUuSz+6    
                        pry+jfwdMCI7dvDurDucqQAZxLavHUI3BuQNcT4ozdT7Zu0OHgSK1PQYvAFvCQ==
                        -----END CERTIFICATE-----
            """;

    var pemContent = PemContent.of(pem);
    
    pemContent.getCertificates().forEach(System.out::println);
}
/*
TEST OK
*/
@Test
void test2() {
    var pem = """
            -----BEGIN CERTIFICATE-----
            MIHrMIGeoAMCAQICFF4UkhlALvGwUHfuZBulAZDim5rlMAUGAytlcDAVMRMwEQYD
                            VQQDDApwbGF5Z3JvdW5kMCAXDTIzMTIzMTE2MDAwMFoYDzIxMjQxMjMxMTU1OTU5
                    WjAVMRMwEQYDVQQDDApwbGF5Z3JvdW5kMCowBQYDK2VwAyEAl1mgT4NWO98CmI8L
            BfKUV7TU9OQUdBDEOqgLAZyPzQkwBQYDK2VwA0EAm+Xj5Zhqk3tcSWEKEnUuSz+6    
            pry+jfwdMCI7dvDurDucqQAZxLavHUI3BuQNcT4ozdT7Zu0OHgSK1PQYvAFvCQ==
            -----END CERTIFICATE-----
            """;

    var pemContent = PemContent.of(pem);

    pemContent.getCertificates().forEach(System.out::println);
}

my version: spring-boot 3.3.2

@mhalbritter
Copy link
Contributor

I see, thanks for the clarification. The trimming is needed indeed.

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Aug 12, 2024
@wilkinsona
Copy link
Member

We discussed this today and we're going to try stripping out the unwanted whitespace in private PemContent(String text) constructor. We should also check that isPresentInText(String text) works with unwanted whitespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants