[jira] [Commented] (SSHD-1244) Client fails window adjust above Integer.MAX_VALUE

2022-02-07 Thread Ryosuke Kanda (Jira) 


[ 
https://issues.apache.org/jira/browse/SSHD-1244?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17488555#comment-17488555
 ] 

Ryosuke Kanda commented on SSHD-1244:
-

I'm sorry I'm late. I tried.
Window Adjust was successful and was able to send a 4GB file.

Thank you for your prompt response.

 
{code:java}
10:33:13.394 | WARN  | 8]-nio2-thread-8 | o.a.s.c.c.Window                 | 
org.apache.sshd.common.channel.Window                             143 | 
expand(Window[client/remote](SftpChannelSubsystem[id=0, 
recipient=0]-ClientSessionImpl[kada@/192.168.12.222:18022][sftp])) 
window=4294873079 - truncated expanded size (4294901929) to 2147483647{code}
 

> Client fails window adjust above Integer.MAX_VALUE
> --
>
> Key: SSHD-1244
> URL: https://issues.apache.org/jira/browse/SSHD-1244
> Project: MINA SSHD
>  Issue Type: Bug
>Affects Versions: 2.8.0
>Reporter: Ryosuke Kanda
>Assignee: Lyor Goldstein
>Priority: Minor
> Attachments: Main.java
>
>
> If the new window size specified by SSH_MSG_CHANNEL_WINDOW_ADJUST exceeds 
> INT_MAX, it will not be recognized correctly.
> I think the cause is in the following places:
> org.apache.sshd.common.channel.Window.expand（int）
>  
> I am doing machine translation, so please allow it to be unnatural.
>  
> I encountered this issue when I was using ProFTPD as an SFTP server.
> The version of ProFTPD is 1.3.5e.
> The SFTP feature of ProFTPD notifies 2 ^ 32-1 bytes as the initial window 
> size by default.
> I've confirmed that SSHD can handle this without any problems, so I sent a 
> 4GB file to see what happens when Window Adjust is done.
> As a result, a Window Adjust was done and SSHD was unable to handle this 
> successfully.
>  
> I have attached the client implementation to this issue.
> (Maybe a poor implementation ...)
> The console logs, including the debug logs, were too large to attach.
> The parts that are clearly set for the SSH client are as follows.
> ServerKeyVerifier
> HostConfigEntryResolver
> KeyIdentityProvider
> In the log, the part where the error occurred is as follows.
> You can see that SSHD recognizes the new Window size as a negative value.
> {code:java}
> [sshd-SshClient[343f4d3d]-nio2-thread-5] DEBUG 
> org.apache.sshd.sftp.client.impl.DefaultSftpClient$SftpChannelSubsystem - 
> handleWindowAdjust(SftpChannelSubsystem[id=0, 
> recipient=0]-ClientSessionImpl[kada@/192.168.12.222:18022][sftp]) 
> SSH_MSG_CHANNEL_WINDOW_ADJUST window=-94217
> [sshd-SshClient[343f4d3d]-nio2-thread-5] DEBUG 
> org.apache.sshd.common.io.nio2.Nio2Session - 
> handleReadCycleFailure(Nio2Session[local=/0:0:0:0:0:0:0:0:51143, 
> remote=/192.168.12.222:18022]) IllegalArgumentException after 8166700 nanos 
> at read cycle=103401: Negative window size: -94217
> [sshd-SshClient[343f4d3d]-nio2-thread-5] DEBUG 
> org.apache.sshd.common.io.nio2.Nio2Session - 
> exceptionCaught(Nio2Session[local=/0:0:0:0:0:0:0:0:51143, 
> remote=/192.168.12.222:18022]) caught IllegalArgumentException[Negative 
> window size: -94217] - calling handler
> [sshd-SshClient[343f4d3d]-nio2-thread-5] DEBUG 
> org.apache.sshd.client.session.ClientSessionImpl - 
> signalAuthFailure(ClientSessionImpl[kada@/192.168.12.222:18022]) 
> type=IllegalArgumentException, signalled=false, first=false: Negative window 
> size: -94217
> [sshd-SshClient[343f4d3d]-nio2-thread-5] WARN 
> org.apache.sshd.client.session.ClientSessionImpl - 
> exceptionCaught(ClientSessionImpl[kada@/192.168.12.222:18022])[state=Opened] 
> IllegalArgumentException: Negative window size: -94217
> java.lang.IllegalArgumentException: Negative window size: -94217
>     at 
> org.apache.sshd.common.util.ValidateUtils.createFormattedException(ValidateUtils.java:213)
>     at 
> org.apache.sshd.common.util.ValidateUtils.throwIllegalArgumentException(ValidateUtils.java:179)
>     at 
> org.apache.sshd.common.util.ValidateUtils.checkTrue(ValidateUtils.java:162)
>     at org.apache.sshd.common.channel.Window.expand(Window.java:123)
>     at 
> org.apache.sshd.common.channel.AbstractChannel.handleWindowAdjust(AbstractChannel.java:894)
>     at 
> org.apache.sshd.client.channel.AbstractClientChannel.handleWindowAdjust(AbstractClientChannel.java:448)
>     at 
> org.apache.sshd.common.session.helpers.AbstractConnectionService.channelWindowAdjust(AbstractConnectionService.java:614)
>     at 
> org.apache.sshd.common.session.helpers.AbstractConnectionService.process(AbstractConnectionService.java:477)
>     at 
> org.apache.sshd.common.session.helpers.AbstractSession.doHandleMessage(AbstractSession.java:526)
>     at 
> org.apache.sshd.common.session.helpers.AbstractSession.handleMessage(AbstractSession.java:452)
>     at 
> org.apache.sshd.common.session.helpers.AbstractSession.decode(AbstractSession.java:1524)
>     at 
>

[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2022-02-07 Thread Jira 


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17488507#comment-17488507
 ] 

Emmanuel Lécharny commented on DIRMINA-1107:


backported to 2.0 branch

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Assignee: Jonathan Valliere
>Priority: Major
> Fix For: 2.1.3, 2.0.23
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

[jira] [Updated] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2022-02-07 Thread Jira 


 [ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Emmanuel Lécharny updated DIRMINA-1107:
---
Fix Version/s: 2.0.23

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Assignee: Jonathan Valliere
>Priority: Major
> Fix For: 2.1.3, 2.0.23
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

Re: Chage in NioDatagramAcceptor in 2.1 vs 2.0

2022-02-07 Thread Emmanuel Lécharny 

Yeah, my point exactly, but wanted to be sure.

Thanks  Jonathan !

On 08/02/2022 01:15, Jonathan Valliere wrote:

  Actually, looks like I fixed something unrelated to DIRMINA-996 and marked
it 996 for some reason.

On Feb 7, 2022 at 5:43:49 PM, Emmanuel Lécharny  wrote:


The thing is that I can't see how it the commit is related to the
problem described in the JIRA...


On 07/02/2022 19:24, Jonathan Valliere wrote:

Mark fixed


On Mon, Feb 7, 2022 at 12:14 PM Emmanuel Lécharny mailto:elecha...@gmail.com>> wrote:


 Hi Jonathan,


 the commit 896b170d8d7c0769bca171f0fbe7de9b13a65968 is commented as a

 fix for DIRMINA-996

 (
https://github.com/apache/mina/commit/896b170d8d7c0769bca171f0fbe7de9b13a65968

 <
https://github.com/apache/mina/commit/896b170d8d7c0769bca171f0fbe7de9b13a65968

)



 Is it the case?


 If so, can we close the issue and mark it fixed (note: the patch has

 been applied to 2.1 and 2.2 branches).


 Thanks !


 --

 *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE

 <
https://www.google.com/maps/search/205+Promenade+des+Anglais+%E2%80%93+06200+NICE?entry=gmail=g




 T. +33 (0)4 89 97 36 50

 P. +33 (0)6 08 33 32 61

 emmanuel.lecha...@busit.com 

 https://www.busit.com/ 


 -

 To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org

 

 For additional commands, e-mail: dev-h...@mina.apache.org

 



--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecha...@busit.com https://www.busit.com/





--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecha...@busit.com https://www.busit.com/

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

Re: Chage in NioDatagramAcceptor in 2.1 vs 2.0

2022-02-07 Thread Jonathan Valliere 
 Actually, looks like I fixed something unrelated to DIRMINA-996 and marked
it 996 for some reason.

On Feb 7, 2022 at 5:43:49 PM, Emmanuel Lécharny  wrote:

> The thing is that I can't see how it the commit is related to the
> problem described in the JIRA...
>
>
> On 07/02/2022 19:24, Jonathan Valliere wrote:
>
> Mark fixed
>
>
> On Mon, Feb 7, 2022 at 12:14 PM Emmanuel Lécharny 
> > wrote:
>
>
> Hi Jonathan,
>
>
> the commit 896b170d8d7c0769bca171f0fbe7de9b13a65968 is commented as a
>
> fix for DIRMINA-996
>
> (
> https://github.com/apache/mina/commit/896b170d8d7c0769bca171f0fbe7de9b13a65968
>
> <
> https://github.com/apache/mina/commit/896b170d8d7c0769bca171f0fbe7de9b13a65968
> >)
>
>
> Is it the case?
>
>
> If so, can we close the issue and mark it fixed (note: the patch has
>
> been applied to 2.1 and 2.2 branches).
>
>
> Thanks !
>
>
> --
>
> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
>
> <
> https://www.google.com/maps/search/205+Promenade+des+Anglais+%E2%80%93+06200+NICE?entry=gmail=g
> >
>
> T. +33 (0)4 89 97 36 50
>
> P. +33 (0)6 08 33 32 61
>
> emmanuel.lecha...@busit.com 
>
> https://www.busit.com/ 
>
>
> -
>
> To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
>
> 
>
> For additional commands, e-mail: dev-h...@mina.apache.org
>
> 
>
>
>
> --
> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
> T. +33 (0)4 89 97 36 50
> P. +33 (0)6 08 33 32 61
> emmanuel.lecha...@busit.com https://www.busit.com/
>

[jira] [Updated] (DIRMINA-1123) Receive buffer size is never set for NIO acceptor

2022-02-07 Thread Jira 


 [ 
https://issues.apache.org/jira/browse/DIRMINA-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Emmanuel Lécharny updated DIRMINA-1123:
---
Fix Version/s: 2.0.23

> Receive buffer size is never set for NIO acceptor
> -
>
> Key: DIRMINA-1123
> URL: https://issues.apache.org/jira/browse/DIRMINA-1123
> Project: MINA
>  Issue Type: Bug
>  Components: Transport
>Affects Versions: 2.0.22, 2.1.3
>Reporter: Marcin L
>Priority: Minor
> Fix For: 2.0.23, 2.1.4
>
> Attachments: case dumps.png, case1.pcap, case2.pcap, case3.pcap
>
>
> Acceptor window size can't be increased beyond OS defaults.
> It seems the receive buffer size is properly set for 
> org.apache.mina.transport.socket.nio.NioSocketConnector, but it is not set at 
> all for org.apache.mina.transport.socket.nio.NioSocketAcceptor before socket 
> is bound.
> org.apache.mina.core.polling.AbstractPollingIoAcceptor.Acceptor#registerHandles
>  comment states that receive buffer size should be initialised, but then 
> org.apache.mina.transport.socket.nio.NioSocketAcceptor#open does not do it.,



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

[jira] [Reopened] (DIRMINA-1123) Receive buffer size is never set for NIO acceptor

2022-02-07 Thread Jira 


 [ 
https://issues.apache.org/jira/browse/DIRMINA-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Emmanuel Lécharny reopened DIRMINA-1123:

  Assignee: (was: Jonathan Valliere)

Reopened to backport the fix to 2.0.X branch

> Receive buffer size is never set for NIO acceptor
> -
>
> Key: DIRMINA-1123
> URL: https://issues.apache.org/jira/browse/DIRMINA-1123
> Project: MINA
>  Issue Type: Bug
>  Components: Transport
>Affects Versions: 2.0.22, 2.1.3
>Reporter: Marcin L
>Priority: Minor
> Fix For: 2.1.4
>
> Attachments: case dumps.png, case1.pcap, case2.pcap, case3.pcap
>
>
> Acceptor window size can't be increased beyond OS defaults.
> It seems the receive buffer size is properly set for 
> org.apache.mina.transport.socket.nio.NioSocketConnector, but it is not set at 
> all for org.apache.mina.transport.socket.nio.NioSocketAcceptor before socket 
> is bound.
> org.apache.mina.core.polling.AbstractPollingIoAcceptor.Acceptor#registerHandles
>  comment states that receive buffer size should be initialised, but then 
> org.apache.mina.transport.socket.nio.NioSocketAcceptor#open does not do it.,



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

Re: Chage in NioDatagramAcceptor in 2.1 vs 2.0

2022-02-07 Thread Emmanuel Lécharny 
The thing is that I can't see how it the commit is related to the 
problem described in the JIRA...



On 07/02/2022 19:24, Jonathan Valliere wrote:

Mark fixed

On Mon, Feb 7, 2022 at 12:14 PM Emmanuel Lécharny > wrote:


Hi Jonathan,

the commit 896b170d8d7c0769bca171f0fbe7de9b13a65968 is commented as a
fix for DIRMINA-996

(https://github.com/apache/mina/commit/896b170d8d7c0769bca171f0fbe7de9b13a65968

)

Is it the case?

If so, can we close the issue and mark it fixed (note: the patch has
been applied to 2.1 and 2.2 branches).

Thanks !

-- 
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE



T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecha...@busit.com 
https://www.busit.com/ 

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org

For additional commands, e-mail: dev-h...@mina.apache.org




--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecha...@busit.com https://www.busit.com/

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

Re: Chage in NioDatagramAcceptor in 2.1 vs 2.0

2022-02-07 Thread Jonathan Valliere 
Mark fixed

On Mon, Feb 7, 2022 at 12:14 PM Emmanuel Lécharny 
wrote:

> Hi Jonathan,
>
> the commit 896b170d8d7c0769bca171f0fbe7de9b13a65968 is commented as a
> fix for DIRMINA-996
> (
> https://github.com/apache/mina/commit/896b170d8d7c0769bca171f0fbe7de9b13a65968
> )
>
> Is it the case?
>
> If so, can we close the issue and mark it fixed (note: the patch has
> been applied to 2.1 and 2.2 branches).
>
> Thanks !
>
> --
> *Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
> 
> T. +33 (0)4 89 97 36 50
> P. +33 (0)6 08 33 32 61
> emmanuel.lecha...@busit.com https://www.busit.com/
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
> For additional commands, e-mail: dev-h...@mina.apache.org
>
>

Chage in NioDatagramAcceptor in 2.1 vs 2.0

2022-02-07 Thread Emmanuel Lécharny 

Hi Jonathan,

the commit 896b170d8d7c0769bca171f0fbe7de9b13a65968 is commented as a 
fix for DIRMINA-996 
(https://github.com/apache/mina/commit/896b170d8d7c0769bca171f0fbe7de9b13a65968)


Is it the case?

If so, can we close the issue and mark it fixed (note: the patch has 
been applied to 2.1 and 2.2 branches).


Thanks !

--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecha...@busit.com https://www.busit.com/

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org