ctt-gob-es / clienteafirma

Cliente @firma
http://administracionelectronica.gob.es/ctt/clienteafirma
247 stars 120 forks source link

Evita NullPointerException al buscar keystore Firefox en algunos casos en Linux #313

Open albfernandez opened 1 year ago

albfernandez commented 1 year ago

Error a resolver

NullPointerException en MozillaKeyStoreUtilitiesUnix.searchLastFirefoxVersion en algunos casos.

Resumen

Si intentamos buscar la versión de Firefox en un directorio que no tenemos permisos pero si que vemos, entonces da un NullPointerException, ya que el método isDirectory nos devuelve true pero el método list nos devuelve null, y se está suponiendo erróneamente que si isDirectory devuelve true, list devolverá un array

if (directoryList.isDirectory() {
    // ...
    for (final String filename : directoryLib.list()) { // NPE aqui porque list() devuelve null
        // ...
    }
}

Relacionado con #252

Detalles

Tenemos un directorio de otro usuario, del que no tenemos permisos, ejemplo:

drwx------  2 root    root    4,0K mar  3 01:02 prueba

Si ejecutamos un programa con otro usuario que no sea root, el método

file.isDirectory();

nos devuelve true, porque el directorio existe.

Pero al ejecutar

file.list();

Nos devuelve null.

Si bien la documentación de Java nos puede dar lugar a confusión, puesto que da a entender lo que pone el código actual: que si isDirectory()devuelve true, list() no devolverá null. Sin embargo en este caso especial no es así, y es necesario poner la comprobación de nulo propuesta en esta Pull Request.

If this abstract pathname does not denote a directory, then this method returns null. 
Otherwise an array of strings is returned, one for each file or directory in the directory. 

traza en la versión 1.7.1

Exception in thread "main" java.lang.ExceptionInInitializerError
at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilities.getSystemNSSLibDir(MozillaKeyStoreUtilities.java:251)
at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilities.loadNSS(MozillaKeyStoreUtilities.java:694)
at es.gob.afirma.keystores.mozilla.NssKeyStoreManager.getNssProvider(NssKeyStoreManager.java:122)
at es.gob.afirma.keystores.mozilla.NssKeyStoreManager.init(NssKeyStoreManager.java:59)
at es.gob.afirma.keystores.mozilla.MozillaUnifiedKeyStoreManager.init(MozillaUnifiedKeyStoreManager.java:75)
at es.gob.afirma.keystores.AOKeyStoreManagerFactory.getNssKeyStoreManager(AOKeyStoreManagerFactory.java:511)
at es.gob.afirma.keystores.AOKeyStoreManagerFactory.getMozillaUnifiedKeyStoreManager(AOKeyStoreManagerFactory.java:542)
at es.gob.afirma.keystores.AOKeyStoreManagerFactory.getAOKeyStoreManager(AOKeyStoreManagerFactory.java:133)
at es.gob.afirma.standalone.SimpleAfirma.main(SimpleAfirma.java:708)
Caused by: java.lang.NullPointerException
at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilitiesUnix.searchLastFirefoxVersion(MozillaKeyStoreUtilitiesUnix.java:145)
at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilitiesUnix.getNssPaths(MozillaKeyStoreUtilitiesUnix.java:78)
at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilitiesUnix.(MozillaKeyStoreUtilitiesUnix.java:26)
... 9 more
clawgrip commented 1 year ago

Alberto, ¿Y si además de comprobar isDirectory() se comprueba canRead() es correcto suponer que .list() nunca va a dar null?

Gracias!


De: Alberto Fernández @.> Enviado: viernes, 3 de marzo de 2023 1:28 Para: ctt-gob-es/clienteafirma @.> Cc: Subscribed @.***> Asunto: [ctt-gob-es/clienteafirma] Evita NullPointerException al buscar keystore Firefox en algunos casos en Linux (PR #313)

Error a resolver

NullPointerException en MozillaKeyStoreUtilitiesUnix.searchLastFirefoxVersion en algunos casos.

Resumen

Si intentamos buscar la versión de Firefox en un directorio que no tenemos permisos pero si que vemos, entonces da un NullPointerException, ya que el método isDirectory nos devuelve true pero el método list nos devuelve null, y se está suponiendo erróneamente que si isDirectory devuelve true, list devolverá un array

if (directoryList.isDirectory() {

// ...

for (final String filename : directoryLib.list()) { // NPE aqui porque list() devuelve null

    // ...

}

}

Relacionado con #252https://github.com/ctt-gob-es/clienteafirma/issues/252

Detalles

Tenemos un directorio de otro usuario, del que no tenemos permisos, ejemplo:

drwx------ 2 root root 4,0K mar 3 01:02 prueba

Si ejecutamos un programa con otro usuario que no sea root, el método

file.isDirectory();

nos devuelve true, porque el directorio existe.

Pero al ejecutar

file.list();

Nos devuelve null.

Si bien la documentación de Java nos puede dar lugar a confusión, puesto que da a entender lo que pone el código actual: que si isDirectory()devuelve true, list() no devolverá null. Sin embargo en este caso especial no es así, y es necesario poner la comprobación de nulo propuesta en esta Pull Request.

If this abstract pathname does not denote a directory, then this method returns null.

Otherwise an array of strings is returned, one for each file or directory in the directory.

traza en la versión 1.7.1

Exception in thread "main" java.lang.ExceptionInInitializerError

at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilities.getSystemNSSLibDir(MozillaKeyStoreUtilities.java:251)

at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilities.loadNSS(MozillaKeyStoreUtilities.java:694)

at es.gob.afirma.keystores.mozilla.NssKeyStoreManager.getNssProvider(NssKeyStoreManager.java:122)

at es.gob.afirma.keystores.mozilla.NssKeyStoreManager.init(NssKeyStoreManager.java:59)

at es.gob.afirma.keystores.mozilla.MozillaUnifiedKeyStoreManager.init(MozillaUnifiedKeyStoreManager.java:75)

at es.gob.afirma.keystores.AOKeyStoreManagerFactory.getNssKeyStoreManager(AOKeyStoreManagerFactory.java:511)

at es.gob.afirma.keystores.AOKeyStoreManagerFactory.getMozillaUnifiedKeyStoreManager(AOKeyStoreManagerFactory.java:542)

at es.gob.afirma.keystores.AOKeyStoreManagerFactory.getAOKeyStoreManager(AOKeyStoreManagerFactory.java:133)

at es.gob.afirma.standalone.SimpleAfirma.main(SimpleAfirma.java:708)

Caused by: java.lang.NullPointerException

at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilitiesUnix.searchLastFirefoxVersion(MozillaKeyStoreUtilitiesUnix.java:145)

at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilitiesUnix.getNssPaths(MozillaKeyStoreUtilitiesUnix.java:78)

at es.gob.afirma.keystores.mozilla.MozillaKeyStoreUtilitiesUnix.(MozillaKeyStoreUtilitiesUnix.java:26)

... 9 more


You can view, comment on, or merge this pull request online at:

https://github.com/ctt-gob-es/clienteafirma/pull/313

Commit Summary

File Changes

(1 filehttps://github.com/ctt-gob-es/clienteafirma/pull/313/files)

Patch Links:

— Reply to this email directly, view it on GitHubhttps://github.com/ctt-gob-es/clienteafirma/pull/313, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABM5YHDR46RUI3MTGRKSHBTW2E3MTANCNFSM6AAAAAAVOAH4TU. You are receiving this because you are subscribed to this thread.Message ID: @.***>

albfernandez commented 1 year ago

Hola Acabo de probarlo y si, con canRead valdría, pero si lo ponemos así mejor poner canExecute también: con canRead podríamos listar los subdirectorios pero si no tenemos canExecute nos devolverá una lista de sub-directorios a los que no podemos acceder.

if (directoryLib.isDirectory() && directoryLib.canRead() && directoryLib.canExecute()){
    // ..
}
clawgrip commented 1 year ago

¡Gracias!


De: Alberto Fernández @.> Enviado: viernes, 3 de marzo de 2023 13:25 Para: ctt-gob-es/clienteafirma @.> Cc: ClawGrip @.>; Comment @.> Asunto: Re: [ctt-gob-es/clienteafirma] Evita NullPointerException al buscar keystore Firefox en algunos casos en Linux (PR #313)

Hola Acabo de probarlo y si, con canRead valdría, pero si lo ponemos así mejor poner canExecute también: con canRead podríamos listar los subdirectorios pero si no tenemos canExecute nos devolverá una lista de sub-directorios a los que no podemos acceder.

— Reply to this email directly, view it on GitHubhttps://github.com/ctt-gob-es/clienteafirma/pull/313#issuecomment-1453458122, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABM5YHED4IF3J5HZPV5PB2TW2HPMXANCNFSM6AAAAAAVOAH4TU. You are receiving this because you commented.Message ID: @.***>

albfernandez commented 1 year ago

Actualizada la Pull Request