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

MuxPool not honoring a timeout of 0 #414

Closed
chhil opened this issue May 27, 2021 · 2 comments · May be fixed by #415
Closed

MuxPool not honoring a timeout of 0 #414

chhil opened this issue May 27, 2021 · 2 comments · May be fixed by #415

Comments

@chhil
Copy link
Contributor

chhil commented May 27, 2021

MuxPool request method
https://github.com/jpos/jPOS/blob/master/jpos/src/main/java/org/jpos/q2/iso/MUXPool.java#L72-L82

If timeout is 0. (I don't want to set a mux timeout, let it take as much time as it wants).

long maxWait = System.currentTimeMillis() + timeout;
followed by 
timeout = maxWait - System.currentTimeMillis();

timeout will return a negative timeout (or on fast systems the System.currentTimeMillis() may not change due to OS timer precision and result in timeout of 0). Currently a null is being returned (see snippet below)

        if (mux != null) {
            timeout = maxWait - System.currentTimeMillis();// maxwait is the previous call to System.currentmilli
            if (timeout >= 0)
                return mux.request (m, timeout);
        }
        return null;

My thoughts on this are:
If a timeout passed to the muxpool.request is 0, means we don't care how much time is taken in the muxpool and the 0 timeout must be passed to the underlying mux.

Additionally, when timeout of 0 is passed to the muxpool the call :
MUX mux = getMUX(m,maxWait);
The snippet below will result in the mux selection being called only once.

} while (System.currentTimeMillis() < maxWait); // maxwait is the system.currentmill and while loop will terminate after looping once as System.currentTimeMillis() will always be greater than maxWait.

So, a decision needs to be made on supporting a timeout of 0 and how long do we want to loop in getting the mux if none are available when timeout is 0.

@chhil
Copy link
Contributor Author

chhil commented May 27, 2021

The above is based on a slack conversation
https://jpos.slack.com/archives/CHAB8RFKK/p1621959937026900

There is also muxpool.send issue stated by Jason Hilliard.
https://jpos.slack.com/archives/CHAB8RFKK/p1622066968035200?thread_ts=1621959937.026900&cid=CHAB8RFKK

It kind of states a similar thing to looping once to get a mux when the muxpool.send method is used.

@jameshilliard
Copy link
Contributor

Potential fix in #415.

@ar ar closed this as completed in 4cd6c95 May 28, 2021
don4rolex added a commit to hextremelabs/jPOS that referenced this issue Sep 12, 2022
* adding check-connected flag to QueryHost

* BERTLVPackager can't pack non EMVStandardTagType

attempt to fix jpos#349

* Update gradle wrapper version to version 6.7

* add EnvironmentProvider support

* Add OBF command

* add ability to read LMKs from the environment

* Adjust int length

* use two colons in file prefix to avoid conflicts

* Use two colons in prefix

* bug 355: fix MAX_TAG_BYTES constant to support 3 bytes.

In the project we could have a tag id constructed of 3 bytes. This patch
allows us to handle it correctly.

* Refactor Q2 to use NIO API for file operations without changing
existing public interfaces.

* Revert "Refactor Q2 to use NIO API for file operations without changing existing public interfaces."

* Upgrade JLine to 3.17.1

* VFS2 does not belong here

* Add support for YAML-based property files

jPOS supports the following standard property element:

   <property file="cfg/xxx.cfg" />

with this change, if the properties file ends in `.yml`, we parse it
as a YAML based configuration.

* properly dereference property file value

* update jPOS version in doc

[ci skip]

* minor code reformat - indents

[ci skip]

* Calculate CVV CLI command

[ci skip]

* send buffered events in background executor

* Welcome Matias Peyroti !

[ci skip]

* remove '@' [ci skip]

* Thank you Josip!

[ci skip]

* Update Track2.java

Sometimes "4111111111111111=" is a valid track2

* Update CardTest.java

* cmf packager: add DEs for PIN change

* add cvkB

* It's actually a CVK, not PVK

[ci skip]

* removed old commented-out regexp

[ci skip]

* Allow environment overrides

A property named `xx.yy.zz` can be overriden by the environment variable `XX_YY_ZZ`.

* do not leak plaintext's exact length

* Update ISOUtil.java

* Update ISOUtil.java


1. Added new definition for hex2byte. 
public static byte[] hex2byte (String s, Charset charset) {
        if (charset == null) charset = CHARSET;
        if (s.length() % 2 == 0) {
            return hex2byte (s.getBytes(charset), 0, s.length() >> 1);
        } else {
        	// Padding left zero to make it even size #Bug raised by tommy
        	return hex2byte("0"+s, charset);
        }
    }

2. Changed hex2byte (String s) to 
public static byte[] hex2byte (String s) {
        return hex2byte (s, CHARSET);
    }

* Thank you Keshaw for your contribution !

[ci skip]

* Update Track2.java

Remove extra "?"

* add jpos:dependencyCheckAnalyze target

* update test expiration

* Upgrade BC to 1.67

* Improve JDBMSpace gc() SpaceError handling

It partially improve problem addressed in jpos#311

* fixed typo

* Fix UnsupportedOperationException when with4characterMACVersions used

* Fixes length of MAC when parsing key block

The commit ba36800 introduces ability to parse key block,
but the one of crutial elements is misinterpreted.

* getInTransit() goes back to report head-tail

* Add ACL based permission support to LogRotationTestDirectory.

* Add connection retry to SslChannelIntegrationTest.

* Replace System.currentTimeMillis() with Instant.now().

This appears to improve timer accuracy on non-Linux platforms.

* jPOS release 2.1.5 [ci skip]

* new development version is 2.1.6-SNAPSHOT

* Make RotateLogListener AutoCloseable so that we can clean up the test directory properly.

* ThroughputControl: Replace System.currentTimeMillis() with Instant.now().

* TPS: Replace System.currentTimeMillis() with Instant.now().

* Space Tests: Replace System.currentTimeMillis() with Instant.now().

* ThroughputControlTestCase: Replace System.currentTimeMillis() with Instant.now().

* Add github workflow and fix some unreliable tests.

* QMUXTestCase: Improve deploy directory isolation.

* Make copy operations more robust.

* Upgrade JLine to 3.18.0

* DUKPT: support KSN of length > 16

Derivation of current key in DUKPT was assuming KSN always of length 16.

* Classloader creation is a privileged operation

* Force UTF-8

* Compensate for sleep inaccuracy in Github Windows/OSX CI.

* Allow empty defaults in environment references

So you can have `"${propname:}"` and get an empty string instead of `"${propname:}"` if `propname` not defined in environment

* Add test for empty Environment default

* Better EBCDIC handling of DE-48.0 (TCC) for Mastercard

* fix NPE

* Add -XX:+IgnoreUnrecognizedVMOptions to bin/q2.

* update COPYRIGHT year

* Upgrade JLine to 3.19.0

* ChannelAdaptor respect 'enabled' on filters config

The beans created with the QFactory should respect the `enabled`
attribute in the definition XML, this commit add this control to the
ChannelAdaptor ISO filters.

Signed-off-by: Arturo Volpe <avolpe@fintech.works>

* Add Environment support for OneShotChannelAdaptorMK2

The OneShotChannelAdaptorMK2 uses various properties that are passed as
children in the bean configuration, to allow a runtime configuration of
those values (and to share the same behaviour as the
OneShotChannelAdaptor) we use the new Environment singleton to get the
property.

Signed-off-by: Arturo Volpe <avolpe@fintech.works>

* Upgrade Gradle wrapper to 6.8.2

* To expose session increase logic

* Minimize number of stat calls

* Thank you for your contribution Sebastien

[ci skip]

* Graduated SensitiveString from jPOS-EE (org.jpos.crypto)

as suggested from fgonzal

* use double-colons - sugggested rewording by BBB

[ci skip]

* Upgrade OWASP dependency check to 6.1.1

* remove spotbugs for now

* Upgrade Mockito to 3.8.0

* add GenericPackagerParams - support TLV/LTV encoded subfields

* Caller not used

* jPOS release 2.1.6 [ci skip]

* New development version 2.1.7-SNAPSHOT

* fixing and enhancing docs for Environment config [ci skip]

* ignore .sdkmanrc [ci skip]

* Upgrade JLINE to 3.20.0

* Connection confirmation message for BaseChannel in client mode

* Upgrade to Gradle 7.0.1

* Upgrade BND to 5.3.0, OWASP to 6.1.5

* Upgrade to Gradle 7.0.2

* Add PosDataCode::isCardNotPresent.

* Honor 0 timeout (fire-and-forget)

closes jpos#414

* request methods: If timeout is 0, call send right away

In addition, in async call, expire the response listener
(alghouth it has very little sense to call async with timeout 0)

* fix DirPoll code example in doc

[ci skip]

* Copy LICENSE file from repo root

[ci skip]

* Adds code of conduct from contributor covenant

* Adds contributor covenant badge to README. Also fixes some line breaks

* Fix Code of Conduct URL

* Update jPOS logo

* PR-437

NPE as the check value is never set in the SecureKeyBlockBuilder jpos#437

* Bump SLF4J to 1.7.32

* add handy IgnoreLogListener

This LogListener can be used for example to swallow aggressive 0800 NMS messages

* NameRegister at the end of startService

closes jpos#439

* Adding $cfg{...} syntax

* Adding Environment.getDir()

* Adding command line -Ed/--envdir to Q2 to change the environment dir

* Force Environment.reload() if env options were changed. Fix Q2Test.testGetEnvDir()

* Renaming Environment#getDir() to Environment#getEnvDir()

* add removeField(s)

* use LinkedHashMap to keep order

removed some useless tests

* Swallow SLF4JDynamicBinder triggered in JDK 16 and up

* Oops, revert WIP dynamic bean deployment

* Implement TVR parser.

* Update abort_participant.adoc

* crop image

* Added class QBeanAsyncSupport

Signed-off-by: Bojan Momcilov <bojan.momcilov@yandex.com>

* Welcome Bojan to the CREDITS file!

* Fixed race condition in Context resume

If a Context gets resumed before the TM actually pauses it,
the resumeOnPause indicator was left true. On a second PAUSE
during the same transaction, the context would get auto-resumed.

Bug and fix provided by Ozzy, who says "It was strange to find a bug in jPOS!"

* add hasFailure, hasWarning, hasInfo, hasIRC helpers

* generalize commaEncode/Decode in ISOUtil to use any character as delimiter

* Welcome Pablo !

* validate that track2 lenght is not greater than 37

* Handle PANs shorter than 16

When PAN is short (i.e. 14 digits), discretionary data gets longer.

fixes jpos#449

* honor multiple EnviromentProviders in single property

* improve charEncode/decode to preserve empty or null strings

* fix charEncode when all string are empty or null

* fix charEncode/Decode use StringJoiner

* added javadoc NOTE to charDecode edge case

* fixing indentation in cmf.xml

* fixing field 113.39 in cmf.xml

* Handle 3-byte tags

closes jpos#453

* restrict 3-byte tags to those starting with 0xDF

* add autoconfig support

* Let FSDProtectedLogListener protect FSDMsg ...

instances as well as protecting `FSDISOMsg`'s.

Also, remove unnecessary code.

* Upgrade BC to 1.69

* fixed length

closes jpos#458

* Add support for multiple environments

-E (or --environent) switch can be repeated now, i.e:

   q2 --environment=default --environment=QA

* honor multiple property configs

* handle YAML and properties in the same loop

Thank you Ozzie for the suggestion

* bump JDOM to 2.0.6.1 - fixes CVE-2021-33813

* log4shell prevention

jPOS doesn't use log4j, but here is our 2c to mitigate the problem
in jPOS based applications that might use log4j

* Thank you Cristian for your PRs!

* autoconfigure Booleans too

* AutoConfig ancestors

* relax setup wait

just two seconds was causing problems on heavy loaded systems

* Added move and hasKey helpers

* prevent loop in Envirnoment.get

* Update copyright

* Q2 help clarification for -E

* reenable non-greedy ? from inner regX.

closes jpos#464

* Fixed javadoc to render the examples correctly.

* Document log listeners in jPOS manual

Add `FSDLogListener` too.

* dereference system properties

* Add Environment awareness to the SimpleConfiguration Factory.

* document PR 468

* jPOS release 2.1.7

* new development version is 2.1.8-SNAPSHOT

* Initialize ServiceConfig prior to calling readCongig. Add unit test

* TransactionManager: abort on misconfigured groups

* Adding getBin(len) to Card and CardHolder (support 8-digit-bin)

* Update RotateLogListener.java

update for issue jpos#264 regarding need to use ZoneId at rotate time because it's different on regular events.

* LL part defined as ASCII

closes jpos#480

* Add DCO

* QueryHost force abort when expired in continuations mode

* TransactionManager: give explicit info about forced abort of paused txn

* de-reference environment properties

issues/482

* Honor Environment in some config. Fixes jpos#482

* Honor Environment in some config. Fixes jpos#482

* Add support for CMF DE 027 - POS Capability

* MUXPool.send may block if no connected MUXes

fixes jpos#487

* Minor text edits / fixes.

* Add comment to dump Bitmap in the ISOMsg dump. jpos#488

* Passwords can be de-referenced from the environment

And eventually picked off an HSM. So this implementation is
safe enough for production.

* Update BcdPrefixer.java

add length encoded with 6 BCD digits

* Create IFB_LLLLLLCHAR.java

add 6 BCD digits length FieldPackager

* add IFB_LLLHEX

* Merge upstream and upgrade to jdk 17

* Update source compatibility

* Update jdk version

Signed-off-by: Arturo Volpe <avolpe@fintech.works>
Signed-off-by: Bojan Momcilov <bojan.momcilov@yandex.com>
Co-authored-by: Barzilai Spinak <barspi@transactility.com>
Co-authored-by: Alejandro Revilla <apr@jpos.org>
Co-authored-by: James Hilliard <james.hilliard1@gmail.com>
Co-authored-by: Alexey Kuznetsov <dark1st@mail.ru>
Co-authored-by: Igor Skljar <39621064+it240884sii@users.noreply.github.com>
Co-authored-by: Andrés Alcarraz <aa@transactility.com>
Co-authored-by: keshawsinha <keshaw@phsolutions.in>
Co-authored-by: Robert Demski <drdemsey@gmail.com>
Co-authored-by: Arturo Volpe <avolpe@fintech.works>
Co-authored-by: Sebastien <sblind@marqeta.com>
Co-authored-by: Federico <fg@transactility.com>
Co-authored-by: Josefina Revilla <josefinarevilla@gmail.com>
Co-authored-by: Santiago Revilla <spr@jpos.org>
Co-authored-by: Murtuza Chhil <chhil@users.noreply.github.com>
Co-authored-by: Bojan Momcilov <bojan.momcilov@yandex.com>
Co-authored-by: Pablo Marziotto <pmarziotto@midinero.com.uy>
Co-authored-by: Ozzy E <espaillato@users.noreply.github.com>
Co-authored-by: galihlasahido <galih.lasahido@gmail.com>
Co-authored-by: Gaston-G <gasfgp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants